[Sound-open-firmware] [PATCH V3 1/2] ASoc: SOF: Add memory window for all platform
From: Pan Xiuli xiuli.pan@linux.intel.com
Add memory window handler for BYT, HSW and BDW.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com --- Test with: Mininow max rt5651 GP-MRB nocodec SOF master: 1693b66bb1d804ded975767cc1e5911e6ff9c93c SOF-Tool master: a02abb799405d0e4ad0d6bb46eacf6fbe958c06e https://github.com/plbossart/sound/tree/topic/sof-v4.14: 9513a73b981bc1917705671ec54402a7e21672eb
--- sound/soc/sof/hw-apl.c | 11 +++++-- sound/soc/sof/hw-bdw.c | 86 ++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-byt.c | 87 +++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-hsw.c | 85 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 233 insertions(+), 36 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index bfbdd915..de0efe1 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1089,9 +1089,13 @@ static int apl_prepare(struct snd_sof_dev *sdev, unsigned int format, static void apl_get_windows(struct snd_sof_dev *sdev) { struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; int i; - u32 inbox_offset = 0, outbox_offset = 0; - u32 inbox_size = 0, outbox_size = 0;
if (!sdev->info_window) return; @@ -1135,6 +1139,9 @@ static void apl_get_windows(struct snd_sof_dev *sdev) elem->size, "debug"); break; case SOF_IPC_REGION_STREAM: + stream_offset = + elem->offset + SRAM_WINDOW_OFFSET(elem->id); + stream_size = elem->size; snd_sof_debugfs_create_item(sdev, sdev->bar[APL_DSP_BAR] + elem->offset + diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index bc3141f..5ec0c5b 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = { {"iram", BDW_DSP_BAR, IRAM_OFFSET, BDW_IRAM_SIZE}, {"dram", BDW_DSP_BAR, DRAM_OFFSET, BDW_DRAM_SIZE}, {"shim", BDW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BDW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, };
/* @@ -379,6 +378,75 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void bdw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i; + + if (sdev->info_window == NULL) + return; + + for (i = 0; i < sdev->info_window->num_windows; i++) { + + elem = &sdev->info_window->window[i]; + + switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break; + } + } + + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size); + + dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +} + static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -394,19 +462,15 @@ static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ bdw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready));
- snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size); - - dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size); - dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time);
+ /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready)); + + bdw_get_windows(sdev); + return 0; }
diff --git a/sound/soc/sof/hw-byt.c b/sound/soc/sof/hw-byt.c index 209f72a..031cec6 100644 --- a/sound/soc/sof/hw-byt.c +++ b/sound/soc/sof/hw-byt.c @@ -73,7 +73,6 @@ static const struct snd_sof_debugfs_map byt_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, };
static const struct snd_sof_debugfs_map cht_debugfs[] = { @@ -89,7 +88,6 @@ static const struct snd_sof_debugfs_map cht_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, };
static void byt_dump(struct snd_sof_dev *sdev, u32 flags) @@ -197,6 +195,75 @@ static void byt_block_read(struct snd_sof_dev *sdev, u32 offset, void *dest, /* * IPC Firmware ready. */ +static void byt_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i; + + if (sdev->info_window == NULL) + return; + + for (i = 0; i < sdev->info_window->num_windows; i++) { + + elem = &sdev->info_window->window[i]; + + switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break; + } + } + + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size); + + dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +} + static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -212,19 +279,15 @@ static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ byt_block_read(sdev, offset, fw_ready, sizeof(*fw_ready));
- snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size); - - dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size); - dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time);
+ /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready)); + + byt_get_windows(sdev); + return 0; }
diff --git a/sound/soc/sof/hw-hsw.c b/sound/soc/sof/hw-hsw.c index c6366bb..2f36216 100644 --- a/sound/soc/sof/hw-hsw.c +++ b/sound/soc/sof/hw-hsw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map hsw_debugfs[] = { {"iram", HSW_DSP_BAR, IRAM_OFFSET, HSW_IRAM_SIZE}, {"dram", HSW_DSP_BAR, DRAM_OFFSET, HSW_DRAM_SIZE}, {"shim", HSW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", HSW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, };
/* @@ -380,6 +379,75 @@ static irqreturn_t hsw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void hsw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i; + + if (sdev->info_window == NULL) + return; + + for (i = 0; i < sdev->info_window->num_windows; i++) { + + elem = &sdev->info_window->window[i]; + + switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break; + } + } + + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size); + + dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +} + static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -395,18 +463,13 @@ static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ hsw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready));
- snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size); - - dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size); - dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready)); + + hsw_get_windows(sdev);
return 0; }
From: Pan Xiuli xiuli.pan@linux.intel.com
Use memory window to store postion data, use old IPC as a notify.
Add memory window handler for BYT, HSW, BDW and APL.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com --- Test with: Mininow max rt5651 GP-MRB nocodec SOF master: 1693b66bb1d804ded975767cc1e5911e6ff9c93c SOF-Tool master: a02abb799405d0e4ad0d6bb46eacf6fbe958c06e https://github.com/plbossart/sound/tree/topic/sof-v4.14: 9513a73b981bc1917705671ec54402a7e21672eb
--- sound/soc/sof/hw-apl.c | 4 ++++ sound/soc/sof/hw-bdw.c | 4 ++++ sound/soc/sof/hw-byt.c | 4 ++++ sound/soc/sof/hw-hsw.c | 4 ++++ sound/soc/sof/ipc.c | 40 +++++++++++++++++++++++++++++++++++----- sound/soc/sof/pcm.c | 15 ++++++++++++--- sound/soc/sof/sof-priv.h | 2 ++ 7 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index de0efe1..5a65e72 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1164,11 +1164,15 @@ static void apl_get_windows(struct snd_sof_dev *sdev)
snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size); + sdev->stream_box.offset = stream_offset; + sdev->stream_box.size = stream_size;
dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size); + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n", + stream_offset, stream_size); }
static int apl_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index 5ec0c5b..9bcb0de 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -440,11 +440,15 @@ static void bdw_get_windows(struct snd_sof_dev *sdev)
snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size); + sdev->stream_box.offset = stream_offset; + sdev->stream_box.size = stream_size;
dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size); + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n", + stream_offset, stream_size); }
static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) diff --git a/sound/soc/sof/hw-byt.c b/sound/soc/sof/hw-byt.c index 031cec6..43a4028 100644 --- a/sound/soc/sof/hw-byt.c +++ b/sound/soc/sof/hw-byt.c @@ -257,11 +257,15 @@ static void byt_get_windows(struct snd_sof_dev *sdev)
snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size); + sdev->stream_box.offset = stream_offset; + sdev->stream_box.size = stream_size;
dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size); + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n", + stream_offset, stream_size); }
static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) diff --git a/sound/soc/sof/hw-hsw.c b/sound/soc/sof/hw-hsw.c index 2f36216..b8523b4 100644 --- a/sound/soc/sof/hw-hsw.c +++ b/sound/soc/sof/hw-hsw.c @@ -441,11 +441,15 @@ static void hsw_get_windows(struct snd_sof_dev *sdev)
snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size); + sdev->stream_box.offset = stream_offset; + sdev->stream_box.size = stream_size;
dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size); + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n", + stream_offset, stream_size); }
static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index da427d1..41068a3 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -249,10 +249,15 @@ EXPORT_SYMBOL(snd_sof_dsp_mailbox_init);
static void ipc_period_elapsed(struct snd_sof_dev *sdev, u32 msg_id) { - struct snd_sof_pcm *spcm; struct sof_ipc_stream_posn posn; + struct snd_sof_pcm *spcm; + u32 posn_offset; int direction;
+ /* TODO: virtual BE/FE will still use IPC message. Should still use this + * read back function. + */ +#if 0 /* read back full message */ snd_sof_dsp_mailbox_read(sdev, sdev->dsp_box.offset, &posn, sizeof(posn)); @@ -260,33 +265,56 @@ static void ipc_period_elapsed(struct snd_sof_dev *sdev, u32 msg_id) posn.host_posn, posn.dai_posn, posn.wallclock);
spcm = snd_sof_find_spcm_comp(sdev, posn.comp_id, &direction); +#endif + spcm = snd_sof_find_spcm_comp(sdev, SOF_IPC_MESSAGE_ID(msg_id), + &direction); + if (!spcm) { dev_err(sdev->dev, "error: period elapsed for unknown component %d\n", posn.comp_id); return; }
+ posn_offset = spcm->posn_offset[direction]; + snd_sof_dsp_mailbox_read(sdev, posn_offset, &posn, sizeof(posn)); + + dev_dbg(sdev->dev, "posn mailbox: posn offset is 0x%x", posn_offset); + dev_dbg(sdev->dev, "posn mailbox: host 0x%llx dai 0x%llx wall 0x%llx\n", + posn.host_posn, posn.dai_posn, posn.wallclock); + memcpy(&spcm->stream[direction].posn, &posn, sizeof(posn)); snd_pcm_period_elapsed(spcm->stream[direction].substream); }
static void ipc_xrun(struct snd_sof_dev *sdev, u32 msg_id) { - struct snd_sof_pcm *spcm; struct sof_ipc_stream_posn posn; + struct snd_sof_pcm *spcm; + u32 posn_offset; int direction;
+ /* TODO: virtual BE/FE will still use IPC message. Should still use this + * read back function. + */ + +#if 0 /* read back full message */ snd_sof_dsp_mailbox_read(sdev, sdev->dsp_box.offset, &posn, sizeof(posn));
spcm = snd_sof_find_spcm_comp(sdev, posn.comp_id, &direction); +#endif + spcm = snd_sof_find_spcm_comp(sdev, SOF_IPC_MESSAGE_ID(msg_id), + &direction); if (!spcm) { dev_err(sdev->dev, "error: XRUN for unknown component %d\n", posn.comp_id); return; }
+ posn_offset = spcm->posn_offset[direction]; + snd_sof_dsp_mailbox_read(sdev, posn_offset, &posn, sizeof(posn)); + dev_dbg(sdev->dev, "posn XRUN: host %llx comp %d size %d\n", posn.host_posn, posn.xrun_comp_id, posn.xrun_size);
@@ -298,12 +326,14 @@ static void ipc_xrun(struct snd_sof_dev *sdev, u32 msg_id)
static void ipc_stream_message(struct snd_sof_dev *sdev, u32 msg_id) { - switch (msg_id) { + u32 msg_cmd = msg_id & SOF_CMD_TYPE_MASK; + + switch (msg_cmd) { case SOF_IPC_STREAM_POSITION: ipc_period_elapsed(sdev, msg_id); break; case SOF_IPC_STREAM_TRIG_XRUN: - ipc_xrun(sdev, msg_id); + ipc_xrun(sdev, msg_cmd); break; default: dev_err(sdev->dev, "error: unhandled stream message %x\n", @@ -372,7 +402,7 @@ static void ipc_msgs_rx(struct work_struct *work) case SOF_IPC_GLB_COMP_MSG: break; case SOF_IPC_GLB_STREAM_MSG: - ipc_stream_message(sdev, type); + ipc_stream_message(sdev, hdr.cmd); break; case SOF_IPC_GLB_TRACE_MSG: ipc_trace_message(sdev, type); diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 771625f..129f018 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -53,6 +53,7 @@ static int sof_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_sof_pcm *spcm = rtd->sof; struct sof_ipc_pcm_params pcm; struct sof_ipc_pcm_params_reply ipc_params_reply; + int posn_offset; int ret;
/* nothing todo for BE */ @@ -132,9 +133,6 @@ static int sof_pcm_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
- /* copy offset */ - //spcm->posn_offset[substream->stream] = ipc_params_reply.posn_offset; - /* firmware already configured host stream */ if (ops && ops->host_stream_prepare) { pcm.params.stream_tag = @@ -147,6 +145,17 @@ static int sof_pcm_hw_params(struct snd_pcm_substream *substream, pcm.hdr.cmd, &pcm, sizeof(pcm), &ipc_params_reply, sizeof(ipc_params_reply));
+ /* caclcute offset */ + posn_offset = ipc_params_reply.posn_offset; + if (posn_offset > sdev->stream_box.size && + posn_offset % sizeof(struct sof_ipc_stream_posn) == 0) { + dev_err(sdev->dev, "error: got wrong posn offset 0x%x for PCM %d\n", + posn_offset, ret); + return ret; + } + spcm->posn_offset[substream->stream] = + sdev->stream_box.offset + posn_offset; + return ret; }
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index a096595..ecf0012 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -173,6 +173,7 @@ struct snd_sof_pcm { struct snd_sof_dev *sdev; struct snd_soc_tplg_pcm pcm; struct snd_sof_pcm_stream stream[2]; + u32 posn_offset[2]; struct mutex mutex; /* TODO: not used ? remove ? */ struct list_head list; /* list in sdev pcm list */ }; @@ -321,6 +322,7 @@ struct snd_sof_dev { struct snd_sof_ipc *ipc; struct snd_sof_mailbox dsp_box; /* DSP initiated IPC */ struct snd_sof_mailbox host_box; /* Host initiated IPC */ + struct snd_sof_mailbox stream_box; /* Stream position update */ u64 irq_status; int ipc_irq; u32 next_comp_id; /* monotonic - reset during S3 */
If you ran checkpatch --codespell --strict, you would see this:
WARNING: 'postion' may be misspelled - perhaps 'position'? #6: Use memory window to store postion data, use old IPC as a notify.
CHECK: if this code is redundant consider removing it #109: FILE: sound/soc/sof/ipc.c:260: +#if 0
CHECK: if this code is redundant consider removing it #150: FILE: sound/soc/sof/ipc.c:300: +#if 0
please fix and resubmit.
see also comments below.
On 03/02/2018 03:21 AM, Xiuli Pan wrote:
From: Pan Xiuli xiuli.pan@linux.intel.com
Use memory window to store postion data, use old IPC as a notify.
Add memory window handler for BYT, HSW, BDW and APL.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com
Test with: Mininow max rt5651 GP-MRB nocodec SOF master: 1693b66bb1d804ded975767cc1e5911e6ff9c93c SOF-Tool master: a02abb799405d0e4ad0d6bb46eacf6fbe958c06e https://github.com/plbossart/sound/tree/topic/sof-v4.14: 9513a73b981bc1917705671ec54402a7e21672eb
sound/soc/sof/hw-apl.c | 4 ++++ sound/soc/sof/hw-bdw.c | 4 ++++ sound/soc/sof/hw-byt.c | 4 ++++ sound/soc/sof/hw-hsw.c | 4 ++++ sound/soc/sof/ipc.c | 40 +++++++++++++++++++++++++++++++++++----- sound/soc/sof/pcm.c | 15 ++++++++++++--- sound/soc/sof/sof-priv.h | 2 ++ 7 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index de0efe1..5a65e72 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1164,11 +1164,15 @@ static void apl_get_windows(struct snd_sof_dev *sdev)
snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size);
sdev->stream_box.offset = stream_offset;
sdev->stream_box.size = stream_size;
dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size);
dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n",
stream_offset, stream_size);}
static int apl_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index 5ec0c5b..9bcb0de 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -440,11 +440,15 @@ static void bdw_get_windows(struct snd_sof_dev *sdev)
snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size);
sdev->stream_box.offset = stream_offset;
sdev->stream_box.size = stream_size;
dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size);
dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n",
stream_offset, stream_size);}
static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
diff --git a/sound/soc/sof/hw-byt.c b/sound/soc/sof/hw-byt.c index 031cec6..43a4028 100644 --- a/sound/soc/sof/hw-byt.c +++ b/sound/soc/sof/hw-byt.c @@ -257,11 +257,15 @@ static void byt_get_windows(struct snd_sof_dev *sdev)
snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size);
sdev->stream_box.offset = stream_offset;
sdev->stream_box.size = stream_size;
dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size);
dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n",
stream_offset, stream_size);}
static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
diff --git a/sound/soc/sof/hw-hsw.c b/sound/soc/sof/hw-hsw.c index 2f36216..b8523b4 100644 --- a/sound/soc/sof/hw-hsw.c +++ b/sound/soc/sof/hw-hsw.c @@ -441,11 +441,15 @@ static void hsw_get_windows(struct snd_sof_dev *sdev)
snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size);
sdev->stream_box.offset = stream_offset;
sdev->stream_box.size = stream_size;
dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size);
dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n",
stream_offset, stream_size);}
static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index da427d1..41068a3 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -249,10 +249,15 @@ EXPORT_SYMBOL(snd_sof_dsp_mailbox_init);
static void ipc_period_elapsed(struct snd_sof_dev *sdev, u32 msg_id) {
- struct snd_sof_pcm *spcm; struct sof_ipc_stream_posn posn;
struct snd_sof_pcm *spcm;
u32 posn_offset; int direction;
/* TODO: virtual BE/FE will still use IPC message. Should still use this
virtual has multiple meanings. This could be a stream with no PCM or a stream interfacing with virtio.
* read back function.*/+#if 0 /* read back full message */ snd_sof_dsp_mailbox_read(sdev, sdev->dsp_box.offset, &posn, sizeof(posn)); @@ -260,33 +265,56 @@ static void ipc_period_elapsed(struct snd_sof_dev *sdev, u32 msg_id) posn.host_posn, posn.dai_posn, posn.wallclock);
spcm = snd_sof_find_spcm_comp(sdev, posn.comp_id, &direction); +#endif
- spcm = snd_sof_find_spcm_comp(sdev, SOF_IPC_MESSAGE_ID(msg_id),
&direction);
why can't this be dynamic, if mbox is supported use it else send an IPC to get the info.
if (!spcm) { dev_err(sdev->dev, "error: period elapsed for unknown component %d\n", posn.comp_id); return; }
posn_offset = spcm->posn_offset[direction];
snd_sof_dsp_mailbox_read(sdev, posn_offset, &posn, sizeof(posn));
dev_dbg(sdev->dev, "posn mailbox: posn offset is 0x%x", posn_offset);
dev_dbg(sdev->dev, "posn mailbox: host 0x%llx dai 0x%llx wall 0x%llx\n",
posn.host_posn, posn.dai_posn, posn.wallclock);memcpy(&spcm->stream[direction].posn, &posn, sizeof(posn)); snd_pcm_period_elapsed(spcm->stream[direction].substream); }
static void ipc_xrun(struct snd_sof_dev *sdev, u32 msg_id) {
- struct snd_sof_pcm *spcm; struct sof_ipc_stream_posn posn;
struct snd_sof_pcm *spcm;
u32 posn_offset; int direction;
/* TODO: virtual BE/FE will still use IPC message. Should still use this
* read back function.*/+#if 0 /* read back full message */ snd_sof_dsp_mailbox_read(sdev, sdev->dsp_box.offset, &posn, sizeof(posn));
spcm = snd_sof_find_spcm_comp(sdev, posn.comp_id, &direction); +#endif
- spcm = snd_sof_find_spcm_comp(sdev, SOF_IPC_MESSAGE_ID(msg_id),
&direction);
same here, why can't it be capability-based?
if (!spcm) { dev_err(sdev->dev, "error: XRUN for unknown component %d\n", posn.comp_id); return; }
- posn_offset = spcm->posn_offset[direction];
- snd_sof_dsp_mailbox_read(sdev, posn_offset, &posn, sizeof(posn));
- dev_dbg(sdev->dev, "posn XRUN: host %llx comp %d size %d\n", posn.host_posn, posn.xrun_comp_id, posn.xrun_size);
@@ -298,12 +326,14 @@ static void ipc_xrun(struct snd_sof_dev *sdev, u32 msg_id)
static void ipc_stream_message(struct snd_sof_dev *sdev, u32 msg_id) {
- switch (msg_id) {
- u32 msg_cmd = msg_id & SOF_CMD_TYPE_MASK;
What is the intent of this filter? It's super misleading since the header for ipc_run is :
static void ipc_xrun(struct snd_sof_dev *sdev, u32 msg_id)
and used as:
spcm = snd_sof_find_spcm_comp(sdev, SOF_IPC_MESSAGE_ID(msg_id), &direction);
- switch (msg_cmd) { case SOF_IPC_STREAM_POSITION: ipc_period_elapsed(sdev, msg_id); break; case SOF_IPC_STREAM_TRIG_XRUN:
ipc_xrun(sdev, msg_id);
ipc_xrun(sdev, msg_cmd);@@ -372,7 +402,7 @@ static void ipc_msgs_rx(struct work_struct *work) case SOF_IPC_GLB_COMP_MSG: break; case SOF_IPC_GLB_STREAM_MSG:
ipc_stream_message(sdev, type);
ipc_stream_message(sdev, hdr.cmd);
type = hdr.cmd & SOF_CMD_TYPE_MASK;
static void ipc_stream_message(struct snd_sof_dev *sdev, u32 msg_id) { u32 msg_cmd = msg_id & SOF_CMD_TYPE_MASK;
switch (msg_cmd) { case SOF_IPC_STREAM_POSITION: ipc_period_elapsed(sdev, msg_id); break; case SOF_IPC_STREAM_TRIG_XRUN: ipc_xrun(sdev, msg_cmd); break;
WTH?
break;case SOF_IPC_GLB_TRACE_MSG: ipc_trace_message(sdev, type); diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 771625f..129f018 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -53,6 +53,7 @@ static int sof_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_sof_pcm *spcm = rtd->sof; struct sof_ipc_pcm_params pcm; struct sof_ipc_pcm_params_reply ipc_params_reply;
int posn_offset; int ret;
/* nothing todo for BE */
@@ -132,9 +133,6 @@ static int sof_pcm_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
- /* copy offset */
- //spcm->posn_offset[substream->stream] = ipc_params_reply.posn_offset;
- /* firmware already configured host stream */ if (ops && ops->host_stream_prepare) { pcm.params.stream_tag =
@@ -147,6 +145,17 @@ static int sof_pcm_hw_params(struct snd_pcm_substream *substream, pcm.hdr.cmd, &pcm, sizeof(pcm), &ipc_params_reply, sizeof(ipc_params_reply));
- /* caclcute offset */
calculate
- posn_offset = ipc_params_reply.posn_offset;
- if (posn_offset > sdev->stream_box.size &&
posn_offset % sizeof(struct sof_ipc_stream_posn) == 0) {
I don't get your range checking, it looks puzzling. A combination of logical AND and modulo for a position?
dev_err(sdev->dev, "error: got wrong posn offset 0x%x for PCM %d\n",posn_offset, ret);return ret;- }
- spcm->posn_offset[substream->stream] =
sdev->stream_box.offset + posn_offset;- return ret; }
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index a096595..ecf0012 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -173,6 +173,7 @@ struct snd_sof_pcm { struct snd_sof_dev *sdev; struct snd_soc_tplg_pcm pcm; struct snd_sof_pcm_stream stream[2];
- u32 posn_offset[2]; struct mutex mutex; /* TODO: not used ? remove ? */ struct list_head list; /* list in sdev pcm list */ };
@@ -321,6 +322,7 @@ struct snd_sof_dev { struct snd_sof_ipc *ipc; struct snd_sof_mailbox dsp_box; /* DSP initiated IPC */ struct snd_sof_mailbox host_box; /* Host initiated IPC */
- struct snd_sof_mailbox stream_box; /* Stream position update */ u64 irq_status; int ipc_irq; u32 next_comp_id; /* monotonic - reset during S3 */
On 3/3/2018 09:55, Pierre-Louis Bossart wrote:
If you ran checkpatch --codespell --strict, you would see this:
WARNING: 'postion' may be misspelled - perhaps 'position'? #6: Use memory window to store postion data, use old IPC as a notify.
CHECK: if this code is redundant consider removing it #109: FILE: sound/soc/sof/ipc.c:260: +#if 0
CHECK: if this code is redundant consider removing it #150: FILE: sound/soc/sof/ipc.c:300: +#if 0
please fix and resubmit.
see also comments below.
On 03/02/2018 03:21 AM, Xiuli Pan wrote:
From: Pan Xiuli xiuli.pan@linux.intel.com
Use memory window to store postion data, use old IPC as a notify.
Add memory window handler for BYT, HSW, BDW and APL.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com
Test with: Mininow max rt5651 GP-MRB nocodec SOF master: 1693b66bb1d804ded975767cc1e5911e6ff9c93c SOF-Tool master: a02abb799405d0e4ad0d6bb46eacf6fbe958c06e https://github.com/plbossart/sound/tree/topic/sof-v4.14: 9513a73b981bc1917705671ec54402a7e21672eb
sound/soc/sof/hw-apl.c | 4 ++++ sound/soc/sof/hw-bdw.c | 4 ++++ sound/soc/sof/hw-byt.c | 4 ++++ sound/soc/sof/hw-hsw.c | 4 ++++ sound/soc/sof/ipc.c | 40 +++++++++++++++++++++++++++++++++++----- sound/soc/sof/pcm.c | 15 ++++++++++++--- sound/soc/sof/sof-priv.h | 2 ++ 7 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index de0efe1..5a65e72 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1164,11 +1164,15 @@ static void apl_get_windows(struct snd_sof_dev *sdev) snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size); + sdev->stream_box.offset = stream_offset; + sdev->stream_box.size = stream_size; dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size); + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n", + stream_offset, stream_size); } static int apl_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index 5ec0c5b..9bcb0de 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -440,11 +440,15 @@ static void bdw_get_windows(struct snd_sof_dev *sdev) snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size); + sdev->stream_box.offset = stream_offset; + sdev->stream_box.size = stream_size; dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size); + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n", + stream_offset, stream_size); } static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) diff --git a/sound/soc/sof/hw-byt.c b/sound/soc/sof/hw-byt.c index 031cec6..43a4028 100644 --- a/sound/soc/sof/hw-byt.c +++ b/sound/soc/sof/hw-byt.c @@ -257,11 +257,15 @@ static void byt_get_windows(struct snd_sof_dev *sdev) snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size); + sdev->stream_box.offset = stream_offset; + sdev->stream_box.size = stream_size; dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size); + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n", + stream_offset, stream_size); } static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) diff --git a/sound/soc/sof/hw-hsw.c b/sound/soc/sof/hw-hsw.c index 2f36216..b8523b4 100644 --- a/sound/soc/sof/hw-hsw.c +++ b/sound/soc/sof/hw-hsw.c @@ -441,11 +441,15 @@ static void hsw_get_windows(struct snd_sof_dev *sdev) snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, outbox_offset, outbox_size); + sdev->stream_box.offset = stream_offset; + sdev->stream_box.size = stream_size; dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size); + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n", + stream_offset, stream_size); } static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index da427d1..41068a3 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -249,10 +249,15 @@ EXPORT_SYMBOL(snd_sof_dsp_mailbox_init); static void ipc_period_elapsed(struct snd_sof_dev *sdev, u32 msg_id) { - struct snd_sof_pcm *spcm; struct sof_ipc_stream_posn posn; + struct snd_sof_pcm *spcm; + u32 posn_offset; int direction; + /* TODO: virtual BE/FE will still use IPC message. Should still use this
virtual has multiple meanings. This could be a stream with no PCM or a stream interfacing with virtio.
This is for the virtio part. I will refine the comment.
+ * read back function. + */ +#if 0 /* read back full message */ snd_sof_dsp_mailbox_read(sdev, sdev->dsp_box.offset, &posn, sizeof(posn)); @@ -260,33 +265,56 @@ static void ipc_period_elapsed(struct snd_sof_dev *sdev, u32 msg_id) posn.host_posn, posn.dai_posn, posn.wallclock); spcm = snd_sof_find_spcm_comp(sdev, posn.comp_id, &direction); +#endif + spcm = snd_sof_find_spcm_comp(sdev, SOF_IPC_MESSAGE_ID(msg_id), + &direction);
why can't this be dynamic, if mbox is supported use it else send an IPC to get the info.
That sounds a good idea. Let's discuss with Liam about this new design. + Liam Could we just try to check different memory window to get position info.
if (!spcm) { dev_err(sdev->dev, "error: period elapsed for unknown component %d\n", posn.comp_id); return; } + posn_offset = spcm->posn_offset[direction]; + snd_sof_dsp_mailbox_read(sdev, posn_offset, &posn, sizeof(posn));
+ dev_dbg(sdev->dev, "posn mailbox: posn offset is 0x%x", posn_offset); + dev_dbg(sdev->dev, "posn mailbox: host 0x%llx dai 0x%llx wall 0x%llx\n", + posn.host_posn, posn.dai_posn, posn.wallclock);
memcpy(&spcm->stream[direction].posn, &posn, sizeof(posn)); snd_pcm_period_elapsed(spcm->stream[direction].substream); } static void ipc_xrun(struct snd_sof_dev *sdev, u32 msg_id) { - struct snd_sof_pcm *spcm; struct sof_ipc_stream_posn posn; + struct snd_sof_pcm *spcm; + u32 posn_offset; int direction; + /* TODO: virtual BE/FE will still use IPC message. Should still use this + * read back function. + */
+#if 0 /* read back full message */ snd_sof_dsp_mailbox_read(sdev, sdev->dsp_box.offset, &posn, sizeof(posn)); spcm = snd_sof_find_spcm_comp(sdev, posn.comp_id, &direction); +#endif + spcm = snd_sof_find_spcm_comp(sdev, SOF_IPC_MESSAGE_ID(msg_id), + &direction);
same here, why can't it be capability-based?
if (!spcm) { dev_err(sdev->dev, "error: XRUN for unknown component %d\n", posn.comp_id); return; } + posn_offset = spcm->posn_offset[direction]; + snd_sof_dsp_mailbox_read(sdev, posn_offset, &posn, sizeof(posn));
dev_dbg(sdev->dev, "posn XRUN: host %llx comp %d size %d\n", posn.host_posn, posn.xrun_comp_id, posn.xrun_size); @@ -298,12 +326,14 @@ static void ipc_xrun(struct snd_sof_dev *sdev, u32 msg_id) static void ipc_stream_message(struct snd_sof_dev *sdev, u32 msg_id) { - switch (msg_id) { + u32 msg_cmd = msg_id & SOF_CMD_TYPE_MASK;
What is the intent of this filter? It's super misleading since the header for ipc_run is : static void ipc_xrun(struct snd_sof_dev *sdev, u32 msg_id)
and used as:
spcm = snd_sof_find_spcm_comp(sdev, SOF_IPC_MESSAGE_ID(msg_id), &direction);
Here we just got know which PCM is updated from the IPC message ID. We used the low 16 bit data for the PCM ID and update that PCM's position. Just got some redesign about the IPC header. Should add some comment about this
+ switch (msg_cmd) { case SOF_IPC_STREAM_POSITION: ipc_period_elapsed(sdev, msg_id); break; case SOF_IPC_STREAM_TRIG_XRUN: - ipc_xrun(sdev, msg_id); + ipc_xrun(sdev, msg_cmd); break; default: dev_err(sdev->dev, "error: unhandled stream message %x\n", @@ -372,7 +402,7 @@ static void ipc_msgs_rx(struct work_struct *work) case SOF_IPC_GLB_COMP_MSG: break; case SOF_IPC_GLB_STREAM_MSG: - ipc_stream_message(sdev, type); + ipc_stream_message(sdev, hdr.cmd);
type = hdr.cmd & SOF_CMD_TYPE_MASK;
static void ipc_stream_message(struct snd_sof_dev *sdev, u32 msg_id) { u32 msg_cmd = msg_id & SOF_CMD_TYPE_MASK;
switch (msg_cmd) { case SOF_IPC_STREAM_POSITION: ipc_period_elapsed(sdev, msg_id); break; case SOF_IPC_STREAM_TRIG_XRUN: ipc_xrun(sdev, msg_cmd); break;
WTH?
Some redesign about IPC message header. Did not know how to handle these IPC interface, did you have any idea about these? We need get the ID of the updated PCM to get position info updated.
break; case SOF_IPC_GLB_TRACE_MSG: ipc_trace_message(sdev, type); diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 771625f..129f018 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -53,6 +53,7 @@ static int sof_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_sof_pcm *spcm = rtd->sof; struct sof_ipc_pcm_params pcm; struct sof_ipc_pcm_params_reply ipc_params_reply; + int posn_offset; int ret; /* nothing todo for BE */ @@ -132,9 +133,6 @@ static int sof_pcm_hw_params(struct snd_pcm_substream *substream, return -EINVAL; } - /* copy offset */ - //spcm->posn_offset[substream->stream] = ipc_params_reply.posn_offset;
/* firmware already configured host stream */ if (ops && ops->host_stream_prepare) { pcm.params.stream_tag = @@ -147,6 +145,17 @@ static int sof_pcm_hw_params(struct snd_pcm_substream *substream, pcm.hdr.cmd, &pcm, sizeof(pcm), &ipc_params_reply, sizeof(ipc_params_reply)); + /* caclcute offset */
calculate
+ posn_offset = ipc_params_reply.posn_offset; + if (posn_offset > sdev->stream_box.size && + posn_offset % sizeof(struct sof_ipc_stream_posn) == 0) {
I don't get your range checking, it looks puzzling. A combination of logical AND and modulo for a position?
Liam asked to have a validation about the offset. This check the offset size is overflow or if the offset is not aligned with the structure size. I will add a comment about this check.
+ dev_err(sdev->dev, "error: got wrong posn offset 0x%x for PCM %d\n", + posn_offset, ret); + return ret; + } + spcm->posn_offset[substream->stream] = + sdev->stream_box.offset + posn_offset;
return ret; } diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index a096595..ecf0012 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -173,6 +173,7 @@ struct snd_sof_pcm { struct snd_sof_dev *sdev; struct snd_soc_tplg_pcm pcm; struct snd_sof_pcm_stream stream[2]; + u32 posn_offset[2]; struct mutex mutex; /* TODO: not used ? remove ? */ struct list_head list; /* list in sdev pcm list */ }; @@ -321,6 +322,7 @@ struct snd_sof_dev { struct snd_sof_ipc *ipc; struct snd_sof_mailbox dsp_box; /* DSP initiated IPC */ struct snd_sof_mailbox host_box; /* Host initiated IPC */ + struct snd_sof_mailbox stream_box; /* Stream position update */ u64 irq_status; int ipc_irq; u32 next_comp_id; /* monotonic - reset during S3 */
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On Mon, 2018-03-05 at 14:16 +0800, Pan, Xiuli wrote:
* read back function.*/+#if 0 /* read back full message */ snd_sof_dsp_mailbox_read(sdev, sdev->dsp_box.offset, &posn, sizeof(posn)); @@ -260,33 +265,56 @@ static void ipc_period_elapsed(struct snd_sof_dev *sdev, u32 msg_id) posn.host_posn, posn.dai_posn, posn.wallclock); spcm = snd_sof_find_spcm_comp(sdev, posn.comp_id, &direction); +#endif
- spcm = snd_sof_find_spcm_comp(sdev, SOF_IPC_MESSAGE_ID(msg_id),
&direction);why can't this be dynamic, if mbox is supported use it else send an IPC to get the info.
That sounds a good idea. Let's discuss with Liam about this new design.
- Liam
Could we just try to check different memory window to get position info.
No hard coding as this breaks the single kernel binary..... please do runtime checking for stuff like this.
Liam
On 3/5/2018 21:48, Liam Girdwood wrote:
On Mon, 2018-03-05 at 14:16 +0800, Pan, Xiuli wrote:
* read back function.*/+#if 0 /* read back full message */ snd_sof_dsp_mailbox_read(sdev, sdev->dsp_box.offset, &posn, sizeof(posn)); @@ -260,33 +265,56 @@ static void ipc_period_elapsed(struct snd_sof_dev *sdev, u32 msg_id) posn.host_posn, posn.dai_posn, posn.wallclock); spcm = snd_sof_find_spcm_comp(sdev, posn.comp_id, &direction); +#endif
- spcm = snd_sof_find_spcm_comp(sdev, SOF_IPC_MESSAGE_ID(msg_id),
&direction);why can't this be dynamic, if mbox is supported use it else send an IPC to get the info.
That sounds a good idea. Let's discuss with Liam about this new design.
- Liam
Could we just try to check different memory window to get position info.
No hard coding as this breaks the single kernel binary..... please do runtime checking for stuff like this.
Will refine these cods with dynamic check.
Thanks Xiuli
Liam
On 03/02/2018 03:21 AM, Xiuli Pan wrote:
From: Pan Xiuli xiuli.pan@linux.intel.com
Add memory window handler for BYT, HSW and BDW.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com
Can you please run checkpatch --strict and resubmit. Nothing personal, it's the same for everyone. I have no appetite for fixing this sort of formatting issues myself. See also comments in the code below.
CHECK: Comparison to NULL could be written "!sdev->info_window" #67: FILE: sound/soc/sof/hw-bdw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #71: FILE: sound/soc/sof/hw-bdw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) { +
CHECK: Alignment should match open parenthesis #79: FILE: sound/soc/sof/hw-bdw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #86: FILE: sound/soc/sof/hw-bdw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #91: FILE: sound/soc/sof/hw-bdw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #96: FILE: sound/soc/sof/hw-bdw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #103: FILE: sound/soc/sof/hw-bdw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #108: FILE: sound/soc/sof/hw-bdw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #117: FILE: sound/soc/sof/hw-bdw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #147: FILE: sound/soc/sof/hw-bdw.c:480: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #189: FILE: sound/soc/sof/hw-byt.c:172: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #193: FILE: sound/soc/sof/hw-byt.c:176: + for (i = 0; i < sdev->info_window->num_windows; i++) { +
CHECK: Alignment should match open parenthesis #201: FILE: sound/soc/sof/hw-byt.c:184: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #208: FILE: sound/soc/sof/hw-byt.c:191: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #213: FILE: sound/soc/sof/hw-byt.c:196: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #218: FILE: sound/soc/sof/hw-byt.c:201: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #225: FILE: sound/soc/sof/hw-byt.c:208: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #230: FILE: sound/soc/sof/hw-byt.c:213: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #239: FILE: sound/soc/sof/hw-byt.c:222: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #269: FILE: sound/soc/sof/hw-byt.c:250: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #303: FILE: sound/soc/sof/hw-hsw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #307: FILE: sound/soc/sof/hw-hsw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) { +
CHECK: Alignment should match open parenthesis #315: FILE: sound/soc/sof/hw-hsw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #322: FILE: sound/soc/sof/hw-hsw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #327: FILE: sound/soc/sof/hw-hsw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #332: FILE: sound/soc/sof/hw-hsw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #339: FILE: sound/soc/sof/hw-hsw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #344: FILE: sound/soc/sof/hw-hsw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #353: FILE: sound/soc/sof/hw-hsw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #382: FILE: sound/soc/sof/hw-hsw.c:479: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
Test with: Mininow max rt5651 GP-MRB nocodec SOF master: 1693b66bb1d804ded975767cc1e5911e6ff9c93c SOF-Tool master: a02abb799405d0e4ad0d6bb46eacf6fbe958c06e https://github.com/plbossart/sound/tree/topic/sof-v4.14: 9513a73b981bc1917705671ec54402a7e21672eb
sound/soc/sof/hw-apl.c | 11 +++++-- sound/soc/sof/hw-bdw.c | 86 ++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-byt.c | 87 +++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-hsw.c | 85 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 233 insertions(+), 36 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index bfbdd915..de0efe1 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1089,9 +1089,13 @@ static int apl_prepare(struct snd_sof_dev *sdev, unsigned int format, static void apl_get_windows(struct snd_sof_dev *sdev) { struct sof_ipc_window_elem *elem;
- u32 outbox_offset = 0;
- u32 stream_offset = 0;
- u32 inbox_offset = 0;
- u32 outbox_size = 0;
- u32 stream_size = 0;
- u32 inbox_size = 0; int i;
u32 inbox_offset = 0, outbox_offset = 0;
u32 inbox_size = 0, outbox_size = 0;
if (!sdev->info_window) return;
@@ -1135,6 +1139,9 @@ static void apl_get_windows(struct snd_sof_dev *sdev) elem->size, "debug"); break; case SOF_IPC_REGION_STREAM:
stream_offset =elem->offset + SRAM_WINDOW_OFFSET(elem->id);stream_size = elem->size; snd_sof_debugfs_create_item(sdev, sdev->bar[APL_DSP_BAR] + elem->offset +diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index bc3141f..5ec0c5b 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = { {"iram", BDW_DSP_BAR, IRAM_OFFSET, BDW_IRAM_SIZE}, {"dram", BDW_DSP_BAR, DRAM_OFFSET, BDW_DRAM_SIZE}, {"shim", BDW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE},
{"mbox", BDW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, };
/*
@@ -379,6 +378,75 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) /*
- IPC Firmware ready.
*/ +static void bdw_get_windows(struct snd_sof_dev *sdev) +{
- struct sof_ipc_window_elem *elem;
- u32 outbox_offset = 0;
- u32 stream_offset = 0;
- u32 inbox_offset = 0;
- u32 outbox_size = 0;
- u32 stream_size = 0;
- u32 inbox_size = 0;
- int i;
- if (sdev->info_window == NULL)
return;
with no error?
- for (i = 0; i < sdev->info_window->num_windows; i++) {
elem = &sdev->info_window->window[i];switch (elem->type) {case SOF_IPC_REGION_UPBOX:inbox_offset = elem->offset + MBOX_OFFSET;inbox_size = elem->size;snd_sof_debugfs_create_item(sdev,sdev->bar[BDW_DSP_BAR] + inbox_offset,elem->size, "inbox");break;case SOF_IPC_REGION_DOWNBOX:outbox_offset = elem->offset + MBOX_OFFSET;outbox_size = elem->size;snd_sof_debugfs_create_item(sdev,sdev->bar[BDW_DSP_BAR] + outbox_offset,elem->size, "outbox");break;case SOF_IPC_REGION_TRACE:snd_sof_debugfs_create_item(sdev,sdev->bar[BDW_DSP_BAR] + elem->offset +MBOX_OFFSET, elem->size, "etrace");break;case SOF_IPC_REGION_DEBUG:snd_sof_debugfs_create_item(sdev,sdev->bar[BDW_DSP_BAR] + elem->offset +MBOX_OFFSET, elem->size, "debug");break;case SOF_IPC_REGION_STREAM:stream_offset = elem->offset + MBOX_OFFSET;stream_size = elem->size;snd_sof_debugfs_create_item(sdev,sdev->bar[BDW_DSP_BAR] + stream_offset,elem->size, "stream");break;case SOF_IPC_REGION_REGS:snd_sof_debugfs_create_item(sdev,sdev->bar[BDW_DSP_BAR] + elem->offset +MBOX_OFFSET, elem->size, "regs");break;default:break;
so here you will init a mailbox of size zero? This looks like an error, why continue with the execution flow.
}- }
- snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size,
outbox_offset, outbox_size);- dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n",
inbox_offset, inbox_size);- dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n",
outbox_offset, outbox_size);+}
- static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready;
@@ -394,19 +462,15 @@ static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ bdw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready));
- snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset,
fw_ready->dspbox_size,fw_ready->hostbox_offset,fw_ready->hostbox_size);- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n",
fw_ready->dspbox_offset, fw_ready->dspbox_size);- dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n",
fw_ready->hostbox_offset, fw_ready->hostbox_size);- dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time);
- /* now check for extended data */
- snd_sof_fw_parse_ext_data(sdev,
MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));- bdw_get_windows(sdev);
- return 0; }
diff --git a/sound/soc/sof/hw-byt.c b/sound/soc/sof/hw-byt.c index 209f72a..031cec6 100644 --- a/sound/soc/sof/hw-byt.c +++ b/sound/soc/sof/hw-byt.c @@ -73,7 +73,6 @@ static const struct snd_sof_debugfs_map byt_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE},
{"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, };
static const struct snd_sof_debugfs_map cht_debugfs[] = {
@@ -89,7 +88,6 @@ static const struct snd_sof_debugfs_map cht_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE},
{"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, };
static void byt_dump(struct snd_sof_dev *sdev, u32 flags)
@@ -197,6 +195,75 @@ static void byt_block_read(struct snd_sof_dev *sdev, u32 offset, void *dest, /*
- IPC Firmware ready.
*/ +static void byt_get_windows(struct snd_sof_dev *sdev) +{
- struct sof_ipc_window_elem *elem;
- u32 outbox_offset = 0;
- u32 stream_offset = 0;
- u32 inbox_offset = 0;
- u32 outbox_size = 0;
- u32 stream_size = 0;
- u32 inbox_size = 0;
- int i;
- if (sdev->info_window == NULL)
return;
no error?
- for (i = 0; i < sdev->info_window->num_windows; i++) {
elem = &sdev->info_window->window[i];switch (elem->type) {case SOF_IPC_REGION_UPBOX:inbox_offset = elem->offset + MBOX_OFFSET;inbox_size = elem->size;snd_sof_debugfs_create_item(sdev,sdev->bar[BYT_DSP_BAR] + inbox_offset,elem->size, "inbox");break;case SOF_IPC_REGION_DOWNBOX:outbox_offset = elem->offset + MBOX_OFFSET;outbox_size = elem->size;snd_sof_debugfs_create_item(sdev,sdev->bar[BYT_DSP_BAR] + outbox_offset,elem->size, "outbox");break;case SOF_IPC_REGION_TRACE:snd_sof_debugfs_create_item(sdev,sdev->bar[BYT_DSP_BAR] + elem->offset +MBOX_OFFSET, elem->size, "etrace");break;case SOF_IPC_REGION_DEBUG:snd_sof_debugfs_create_item(sdev,sdev->bar[BYT_DSP_BAR] + elem->offset +MBOX_OFFSET, elem->size, "debug");break;case SOF_IPC_REGION_STREAM:stream_offset = elem->offset + MBOX_OFFSET;stream_size = elem->size;snd_sof_debugfs_create_item(sdev,sdev->bar[BYT_DSP_BAR] + stream_offset,elem->size, "stream");break;case SOF_IPC_REGION_REGS:snd_sof_debugfs_create_item(sdev,sdev->bar[BYT_DSP_BAR] + elem->offset +MBOX_OFFSET, elem->size, "regs");break;default:break;
same, this looks wrong.
}- }
- snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size,
outbox_offset, outbox_size);- dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n",
inbox_offset, inbox_size);- dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n",
outbox_offset, outbox_size);+}
- static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready;
@@ -212,19 +279,15 @@ static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ byt_block_read(sdev, offset, fw_ready, sizeof(*fw_ready));
- snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset,
fw_ready->dspbox_size,fw_ready->hostbox_offset,fw_ready->hostbox_size);- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n",
fw_ready->dspbox_offset, fw_ready->dspbox_size);- dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n",
fw_ready->hostbox_offset, fw_ready->hostbox_size);- dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time);
- /* now check for extended data */
- snd_sof_fw_parse_ext_data(sdev,
MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));- byt_get_windows(sdev);
- return 0; }
diff --git a/sound/soc/sof/hw-hsw.c b/sound/soc/sof/hw-hsw.c index c6366bb..2f36216 100644 --- a/sound/soc/sof/hw-hsw.c +++ b/sound/soc/sof/hw-hsw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map hsw_debugfs[] = { {"iram", HSW_DSP_BAR, IRAM_OFFSET, HSW_IRAM_SIZE}, {"dram", HSW_DSP_BAR, DRAM_OFFSET, HSW_DRAM_SIZE}, {"shim", HSW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE},
{"mbox", HSW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, };
/*
@@ -380,6 +379,75 @@ static irqreturn_t hsw_irq_thread(int irq, void *context) /*
- IPC Firmware ready.
*/ +static void hsw_get_windows(struct snd_sof_dev *sdev) +{
- struct sof_ipc_window_elem *elem;
- u32 outbox_offset = 0;
- u32 stream_offset = 0;
- u32 inbox_offset = 0;
- u32 outbox_size = 0;
- u32 stream_size = 0;
- u32 inbox_size = 0;
- int i;
- if (sdev->info_window == NULL)
return;
no error?
- for (i = 0; i < sdev->info_window->num_windows; i++) {
elem = &sdev->info_window->window[i];switch (elem->type) {case SOF_IPC_REGION_UPBOX:inbox_offset = elem->offset + MBOX_OFFSET;inbox_size = elem->size;snd_sof_debugfs_create_item(sdev,sdev->bar[HSW_DSP_BAR] + inbox_offset,elem->size, "inbox");break;case SOF_IPC_REGION_DOWNBOX:outbox_offset = elem->offset + MBOX_OFFSET;outbox_size = elem->size;snd_sof_debugfs_create_item(sdev,sdev->bar[HSW_DSP_BAR] + outbox_offset,elem->size, "outbox");break;case SOF_IPC_REGION_TRACE:snd_sof_debugfs_create_item(sdev,sdev->bar[HSW_DSP_BAR] + elem->offset +MBOX_OFFSET, elem->size, "etrace");break;case SOF_IPC_REGION_DEBUG:snd_sof_debugfs_create_item(sdev,sdev->bar[HSW_DSP_BAR] + elem->offset +MBOX_OFFSET, elem->size, "debug");break;case SOF_IPC_REGION_STREAM:stream_offset = elem->offset + MBOX_OFFSET;stream_size = elem->size;snd_sof_debugfs_create_item(sdev,sdev->bar[HSW_DSP_BAR] + stream_offset,elem->size, "stream");break;case SOF_IPC_REGION_REGS:snd_sof_debugfs_create_item(sdev,sdev->bar[HSW_DSP_BAR] + elem->offset +MBOX_OFFSET, elem->size, "regs");break;default:break;
same here, this looks wrong.
}- }
- snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size,
outbox_offset, outbox_size);- dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n",
inbox_offset, inbox_size);- dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n",
outbox_offset, outbox_size);+}
- static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready;
@@ -395,18 +463,13 @@ static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ hsw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready));
- snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset,
fw_ready->dspbox_size,fw_ready->hostbox_offset,fw_ready->hostbox_size);- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n",
fw_ready->dspbox_offset, fw_ready->dspbox_size);- dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n",
fw_ready->hostbox_offset, fw_ready->hostbox_size);- dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time);
/* now check for extended data */
snd_sof_fw_parse_ext_data(sdev,
MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));hsw_get_windows(sdev);
return 0; }
And on top on my previous comments this patch causes a kernel oops - likely because the matching patches are not yet merged in the firmware master branch. If there is such a dependency between firmware and driver, please make it clear upfront, thanks.
[ 8.767359] sof-audio sof-audio: ipc: send 0x30010000 [ 9.071043] sof-audio sof-audio: error: ipc timed out for 0x30010000 size 0x3c [ 9.071202] sof-audio sof-audio: error: DSP failed to add widget id 0 type 11 name : PCM0P stream Passthrough Playback reply 0 [ 9.071371] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050 [ 9.071497] IP: sof_widget_unload+0xe/0x90 [snd_sof] [ 9.071569] PGD 0 P4D 0 [ 9.071615] Oops: 0000 [#1] PREEMPT SMP [ 9.071674] Modules linked in: nls_iso8859_1 snd_soc_sst_byt_cht_da7213(+) intel_rapl intel_soc_dts_thermal intel_soc_dts_iosf intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc snd_hda_codec_hdmi aesni_intel aes_x86_64 crypto_simd glue_helper cryptd snd_soc_rt5670 sof_acpi_dev snd_soc_rt5645 snd_hda_intel snd_sof_intel_hsw snd_sof_intel_byt snd_soc_rt5640 snd_soc_acpi_intel_match snd_sof_intel_bdw snd_soc_rl6231 snd_sof snd_soc_acpi snd_soc_da7213 snd_hda_codec lpc_ich snd_hwdep snd_seq_midi mfd_core snd_hda_core snd_seq_midi_event snd_soc_core shpchp snd_compress snd_rawmidi snd_pcm snd_seq snd_seq_device snd_timer snd soundcore 8250_dw ip_tables x_tables autofs4 hid_generic hid_logitech_hidpp hid_logitech_dj usbhid hid mmc_block i915 i2c_algo_bit drm_kms_helper syscopyarea [ 9.072676] sysfillrect sysimgblt fb_sys_fops r8169 drm mii sdhci_acpi video sdhci [ 9.072797] CPU: 0 PID: 250 Comm: systemd-udevd Not tainted 4.14.0-test+ #14 [ 9.072896] Hardware name: ADI Minnowboard Turbot D0 PLATFORM/MinnowBoard Turbot, BIOS MNW2MAX1.X64.0094.R01.1612052239 12/05/2016 [ 9.073054] task: ffff98feb2804240 task.stack: ffffa8cbc06a0000 [ 9.073145] RIP: 0010:sof_widget_unload+0xe/0x90 [snd_sof] [ 9.073223] RSP: 0018:ffffa8cbc06a36a8 EFLAGS: 00010286 [ 9.073300] RAX: ffffffffc053ca60 RBX: 0000000000000000 RCX: 0000000000000001 [ 9.073399] RDX: 0000000000000003 RSI: ffff98feb9cf1ea0 RDI: ffff98feb9e2cc20 [ 9.073497] RBP: ffffa8cbc06a36b0 R08: 0000000000000000 R09: 0000000000000290 [ 9.073596] R10: ffffa8cbc06a3538 R11: 00000000ffffffff R12: ffff98feb9cf1ea0 [ 9.073695] R13: 0000000000000000 R14: ffff98feb9cf1e00 R15: 00000000ffffff92 [ 9.073794] FS: 00007f6ecf192400(0000) GS:ffff98feb8000000(0000) knlGS:0000000000000000 [ 9.073906] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 9.073987] CR2: 0000000000000050 CR3: 0000000036e2e000 CR4: 00000000001006f0 [ 9.074086] Call Trace: [ 9.074148] remove_widget+0x40/0x170 [snd_soc_core] [ 9.074238] snd_soc_tplg_widget_remove.part.22+0x22/0x30 [snd_soc_core] [ 9.074348] soc_tplg_dapm_widget_create+0xd35/0xde0 [snd_soc_core] [ 9.074461] soc_tplg_process_headers+0x77b/0x1f80 [snd_soc_core] [ 9.074554] ? _raw_spin_lock_irqsave+0x1d/0x50 [ 9.074623] ? _raw_spin_unlock_irqrestore+0x18/0x30 [ 9.074698] ? devres_add+0x5f/0x70 [ 9.074755] ? assign_firmware_buf+0x98/0x200 [ 9.074837] snd_soc_tplg_component_load+0x6e/0xa0 [snd_soc_core] [ 9.074932] snd_sof_load_topology+0x60/0xc0 [snd_sof] [ 9.075011] ? __debugfs_create_file+0xce/0x110 [ 9.075084] sof_pcm_probe+0x41/0xa0 [snd_sof] [ 9.075164] snd_soc_platform_drv_probe+0x16/0x20 [snd_soc_core] [ 9.075264] soc_probe_component+0x286/0x3d0 [snd_soc_core] [ 9.075360] snd_soc_register_card+0x636/0xfd0 [snd_soc_core] [ 9.075446] ? __kmalloc_node_track_caller+0xee/0x2b0 [ 9.075537] devm_snd_soc_register_card+0x41/0x80 [snd_soc_core] [ 9.075628] bytcht_da7213_probe+0xb9/0x100 [snd_soc_sst_byt_cht_da7213] [ 9.075725] platform_drv_probe+0x3b/0xa0 [ 9.075789] driver_probe_device+0x2ff/0x450 [ 9.081096] __driver_attach+0xa4/0xe0 [ 9.086348] ? driver_probe_device+0x450/0x450 [ 9.091586] bus_for_each_dev+0x62/0x90 [ 9.096787] driver_attach+0x1e/0x20 [ 9.101990] bus_add_driver+0x1c7/0x270 [ 9.107182] ? 0xffffffffc01ac000 [ 9.112364] driver_register+0x60/0xe0 [ 9.117550] ? 0xffffffffc01ac000 [ 9.122741] __platform_driver_register+0x36/0x40 [ 9.127962] bytcht_da7213_driver_init+0x17/0x1000 [snd_soc_sst_byt_cht_da7213] [ 9.133253] do_one_initcall+0x50/0x190 [ 9.138528] ? do_init_module+0x27/0x203 [ 9.143794] do_init_module+0x5f/0x203 [ 9.149059] load_module+0x259e/0x2d20 [ 9.154307] SYSC_finit_module+0xd7/0xf0 [ 9.159532] ? SYSC_finit_module+0xd7/0xf0 [ 9.164759] SyS_finit_module+0xe/0x10 [ 9.169966] do_syscall_64+0x59/0x110 [ 9.175169] entry_SYSCALL64_slow_path+0x25/0x25 [ 9.180384] RIP: 0033:0x7f6ecea92a49 [ 9.185565] RSP: 002b:00007ffe55acdb88 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 9.190831] RAX: ffffffffffffffda RBX: 00005575ec0f6850 RCX: 00007f6ecea92a49 [ 9.196116] RDX: 0000000000000000 RSI: 00007f6ece77e1c5 RDI: 0000000000000007 [ 9.201427] RBP: 00007f6ece77e1c5 R08: 0000000000000000 R09: 00005575ec0f6850 [ 9.206742] R10: 0000000000000007 R11: 0000000000000246 R12: 0000000000000000 [ 9.212076] R13: 00005575ec0f67f0 R14: 0000000000020000 R15: 00005575ec0f6850 [ 9.217423] Code: 48 05 00 01 00 00 48 89 87 e0 04 00 00 e8 5b ed cb d8 31 c0 5d c3 0f 1f 80 00 00 00 00 0f 1f 44 00 00 55 48 89 e5 53 48 8b 5e 38 <48> 8b 7b 50 48 85 ff 74 38 48 8b 87 90 00 00 00 48 8b 97 88 00 [ 9.223183] RIP: sof_widget_unload+0xe/0x90 [snd_sof] RSP: ffffa8cbc06a36a8 [ 9.228835] CR2: 0000000000000050 [ 9.235140] ---[ end trace d9fd6055d01be8a0 ]---
On 03/02/2018 07:38 PM, Pierre-Louis Bossart wrote:
On 03/02/2018 03:21 AM, Xiuli Pan wrote:
From: Pan Xiuli xiuli.pan@linux.intel.com
Add memory window handler for BYT, HSW and BDW.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com
Can you please run checkpatch --strict and resubmit. Nothing personal, it's the same for everyone. I have no appetite for fixing this sort of formatting issues myself. See also comments in the code below.
CHECK: Comparison to NULL could be written "!sdev->info_window" #67: FILE: sound/soc/sof/hw-bdw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #71: FILE: sound/soc/sof/hw-bdw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #79: FILE: sound/soc/sof/hw-bdw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #86: FILE: sound/soc/sof/hw-bdw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #91: FILE: sound/soc/sof/hw-bdw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #96: FILE: sound/soc/sof/hw-bdw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #103: FILE: sound/soc/sof/hw-bdw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #108: FILE: sound/soc/sof/hw-bdw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #117: FILE: sound/soc/sof/hw-bdw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #147: FILE: sound/soc/sof/hw-bdw.c:480: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #189: FILE: sound/soc/sof/hw-byt.c:172: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #193: FILE: sound/soc/sof/hw-byt.c:176: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #201: FILE: sound/soc/sof/hw-byt.c:184: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #208: FILE: sound/soc/sof/hw-byt.c:191: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #213: FILE: sound/soc/sof/hw-byt.c:196: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #218: FILE: sound/soc/sof/hw-byt.c:201: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #225: FILE: sound/soc/sof/hw-byt.c:208: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #230: FILE: sound/soc/sof/hw-byt.c:213: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #239: FILE: sound/soc/sof/hw-byt.c:222: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #269: FILE: sound/soc/sof/hw-byt.c:250: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #303: FILE: sound/soc/sof/hw-hsw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #307: FILE: sound/soc/sof/hw-hsw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #315: FILE: sound/soc/sof/hw-hsw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #322: FILE: sound/soc/sof/hw-hsw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #327: FILE: sound/soc/sof/hw-hsw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #332: FILE: sound/soc/sof/hw-hsw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #339: FILE: sound/soc/sof/hw-hsw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #344: FILE: sound/soc/sof/hw-hsw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #353: FILE: sound/soc/sof/hw-hsw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #382: FILE: sound/soc/sof/hw-hsw.c:479: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
Test with: Mininow max rt5651 GP-MRB nocodec SOF master: 1693b66bb1d804ded975767cc1e5911e6ff9c93c SOF-Tool master: a02abb799405d0e4ad0d6bb46eacf6fbe958c06e https://github.com/plbossart/sound/tree/topic/sof-v4.14: 9513a73b981bc1917705671ec54402a7e21672eb
sound/soc/sof/hw-apl.c | 11 +++++-- sound/soc/sof/hw-bdw.c | 86 ++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-byt.c | 87 +++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-hsw.c | 85 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 233 insertions(+), 36 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index bfbdd915..de0efe1 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1089,9 +1089,13 @@ static int apl_prepare(struct snd_sof_dev *sdev, unsigned int format, static void apl_get_windows(struct snd_sof_dev *sdev) { struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; int i; - u32 inbox_offset = 0, outbox_offset = 0; - u32 inbox_size = 0, outbox_size = 0; if (!sdev->info_window) return; @@ -1135,6 +1139,9 @@ static void apl_get_windows(struct snd_sof_dev *sdev) elem->size, "debug"); break; case SOF_IPC_REGION_STREAM: + stream_offset = + elem->offset + SRAM_WINDOW_OFFSET(elem->id); + stream_size = elem->size; snd_sof_debugfs_create_item(sdev, sdev->bar[APL_DSP_BAR] + elem->offset + diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index bc3141f..5ec0c5b 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = { {"iram", BDW_DSP_BAR, IRAM_OFFSET, BDW_IRAM_SIZE}, {"dram", BDW_DSP_BAR, DRAM_OFFSET, BDW_DRAM_SIZE}, {"shim", BDW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BDW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -379,6 +378,75 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void bdw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
with no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
so here you will init a mailbox of size zero? This looks like an error, why continue with the execution flow.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -394,19 +462,15 @@ static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ bdw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ bdw_get_windows(sdev);
return 0; } diff --git a/sound/soc/sof/hw-byt.c b/sound/soc/sof/hw-byt.c index 209f72a..031cec6 100644 --- a/sound/soc/sof/hw-byt.c +++ b/sound/soc/sof/hw-byt.c @@ -73,7 +73,6 @@ static const struct snd_sof_debugfs_map byt_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; static const struct snd_sof_debugfs_map cht_debugfs[] = { @@ -89,7 +88,6 @@ static const struct snd_sof_debugfs_map cht_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; static void byt_dump(struct snd_sof_dev *sdev, u32 flags) @@ -197,6 +195,75 @@ static void byt_block_read(struct snd_sof_dev *sdev, u32 offset, void *dest, /* * IPC Firmware ready. */ +static void byt_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
same, this looks wrong.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -212,19 +279,15 @@ static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ byt_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ byt_get_windows(sdev);
return 0; } diff --git a/sound/soc/sof/hw-hsw.c b/sound/soc/sof/hw-hsw.c index c6366bb..2f36216 100644 --- a/sound/soc/sof/hw-hsw.c +++ b/sound/soc/sof/hw-hsw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map hsw_debugfs[] = { {"iram", HSW_DSP_BAR, IRAM_OFFSET, HSW_IRAM_SIZE}, {"dram", HSW_DSP_BAR, DRAM_OFFSET, HSW_DRAM_SIZE}, {"shim", HSW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", HSW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -380,6 +379,75 @@ static irqreturn_t hsw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void hsw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
same here, this looks wrong.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -395,18 +463,13 @@ static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ hsw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ hsw_get_windows(sdev); return 0; }
On 3/3/2018 10:16, Pierre-Louis Bossart wrote:
And on top on my previous comments this patch causes a kernel oops - likely because the matching patches are not yet merged in the firmware master branch. If there is such a dependency between firmware and driver, please make it clear upfront, thanks.
Will resent the patch with cover letter about what the changes about and make some modify to avoid too strict dependency that may make bisect hard.
Thanks Xiuli
[ 8.767359] sof-audio sof-audio: ipc: send 0x30010000 [ 9.071043] sof-audio sof-audio: error: ipc timed out for 0x30010000 size 0x3c [ 9.071202] sof-audio sof-audio: error: DSP failed to add widget id 0 type 11 name : PCM0P stream Passthrough Playback reply 0 [ 9.071371] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050 [ 9.071497] IP: sof_widget_unload+0xe/0x90 [snd_sof] [ 9.071569] PGD 0 P4D 0 [ 9.071615] Oops: 0000 [#1] PREEMPT SMP [ 9.071674] Modules linked in: nls_iso8859_1 snd_soc_sst_byt_cht_da7213(+) intel_rapl intel_soc_dts_thermal intel_soc_dts_iosf intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc snd_hda_codec_hdmi aesni_intel aes_x86_64 crypto_simd glue_helper cryptd snd_soc_rt5670 sof_acpi_dev snd_soc_rt5645 snd_hda_intel snd_sof_intel_hsw snd_sof_intel_byt snd_soc_rt5640 snd_soc_acpi_intel_match snd_sof_intel_bdw snd_soc_rl6231 snd_sof snd_soc_acpi snd_soc_da7213 snd_hda_codec lpc_ich snd_hwdep snd_seq_midi mfd_core snd_hda_core snd_seq_midi_event snd_soc_core shpchp snd_compress snd_rawmidi snd_pcm snd_seq snd_seq_device snd_timer snd soundcore 8250_dw ip_tables x_tables autofs4 hid_generic hid_logitech_hidpp hid_logitech_dj usbhid hid mmc_block i915 i2c_algo_bit drm_kms_helper syscopyarea [ 9.072676] sysfillrect sysimgblt fb_sys_fops r8169 drm mii sdhci_acpi video sdhci [ 9.072797] CPU: 0 PID: 250 Comm: systemd-udevd Not tainted 4.14.0-test+ #14 [ 9.072896] Hardware name: ADI Minnowboard Turbot D0 PLATFORM/MinnowBoard Turbot, BIOS MNW2MAX1.X64.0094.R01.1612052239 12/05/2016 [ 9.073054] task: ffff98feb2804240 task.stack: ffffa8cbc06a0000 [ 9.073145] RIP: 0010:sof_widget_unload+0xe/0x90 [snd_sof] [ 9.073223] RSP: 0018:ffffa8cbc06a36a8 EFLAGS: 00010286 [ 9.073300] RAX: ffffffffc053ca60 RBX: 0000000000000000 RCX: 0000000000000001 [ 9.073399] RDX: 0000000000000003 RSI: ffff98feb9cf1ea0 RDI: ffff98feb9e2cc20 [ 9.073497] RBP: ffffa8cbc06a36b0 R08: 0000000000000000 R09: 0000000000000290 [ 9.073596] R10: ffffa8cbc06a3538 R11: 00000000ffffffff R12: ffff98feb9cf1ea0 [ 9.073695] R13: 0000000000000000 R14: ffff98feb9cf1e00 R15: 00000000ffffff92 [ 9.073794] FS: 00007f6ecf192400(0000) GS:ffff98feb8000000(0000) knlGS:0000000000000000 [ 9.073906] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 9.073987] CR2: 0000000000000050 CR3: 0000000036e2e000 CR4: 00000000001006f0 [ 9.074086] Call Trace: [ 9.074148] remove_widget+0x40/0x170 [snd_soc_core] [ 9.074238] snd_soc_tplg_widget_remove.part.22+0x22/0x30 [snd_soc_core] [ 9.074348] soc_tplg_dapm_widget_create+0xd35/0xde0 [snd_soc_core] [ 9.074461] soc_tplg_process_headers+0x77b/0x1f80 [snd_soc_core] [ 9.074554] ? _raw_spin_lock_irqsave+0x1d/0x50 [ 9.074623] ? _raw_spin_unlock_irqrestore+0x18/0x30 [ 9.074698] ? devres_add+0x5f/0x70 [ 9.074755] ? assign_firmware_buf+0x98/0x200 [ 9.074837] snd_soc_tplg_component_load+0x6e/0xa0 [snd_soc_core] [ 9.074932] snd_sof_load_topology+0x60/0xc0 [snd_sof] [ 9.075011] ? __debugfs_create_file+0xce/0x110 [ 9.075084] sof_pcm_probe+0x41/0xa0 [snd_sof] [ 9.075164] snd_soc_platform_drv_probe+0x16/0x20 [snd_soc_core] [ 9.075264] soc_probe_component+0x286/0x3d0 [snd_soc_core] [ 9.075360] snd_soc_register_card+0x636/0xfd0 [snd_soc_core] [ 9.075446] ? __kmalloc_node_track_caller+0xee/0x2b0 [ 9.075537] devm_snd_soc_register_card+0x41/0x80 [snd_soc_core] [ 9.075628] bytcht_da7213_probe+0xb9/0x100 [snd_soc_sst_byt_cht_da7213] [ 9.075725] platform_drv_probe+0x3b/0xa0 [ 9.075789] driver_probe_device+0x2ff/0x450 [ 9.081096] __driver_attach+0xa4/0xe0 [ 9.086348] ? driver_probe_device+0x450/0x450 [ 9.091586] bus_for_each_dev+0x62/0x90 [ 9.096787] driver_attach+0x1e/0x20 [ 9.101990] bus_add_driver+0x1c7/0x270 [ 9.107182] ? 0xffffffffc01ac000 [ 9.112364] driver_register+0x60/0xe0 [ 9.117550] ? 0xffffffffc01ac000 [ 9.122741] __platform_driver_register+0x36/0x40 [ 9.127962] bytcht_da7213_driver_init+0x17/0x1000 [snd_soc_sst_byt_cht_da7213] [ 9.133253] do_one_initcall+0x50/0x190 [ 9.138528] ? do_init_module+0x27/0x203 [ 9.143794] do_init_module+0x5f/0x203 [ 9.149059] load_module+0x259e/0x2d20 [ 9.154307] SYSC_finit_module+0xd7/0xf0 [ 9.159532] ? SYSC_finit_module+0xd7/0xf0 [ 9.164759] SyS_finit_module+0xe/0x10 [ 9.169966] do_syscall_64+0x59/0x110 [ 9.175169] entry_SYSCALL64_slow_path+0x25/0x25 [ 9.180384] RIP: 0033:0x7f6ecea92a49 [ 9.185565] RSP: 002b:00007ffe55acdb88 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 9.190831] RAX: ffffffffffffffda RBX: 00005575ec0f6850 RCX: 00007f6ecea92a49 [ 9.196116] RDX: 0000000000000000 RSI: 00007f6ece77e1c5 RDI: 0000000000000007 [ 9.201427] RBP: 00007f6ece77e1c5 R08: 0000000000000000 R09: 00005575ec0f6850 [ 9.206742] R10: 0000000000000007 R11: 0000000000000246 R12: 0000000000000000 [ 9.212076] R13: 00005575ec0f67f0 R14: 0000000000020000 R15: 00005575ec0f6850 [ 9.217423] Code: 48 05 00 01 00 00 48 89 87 e0 04 00 00 e8 5b ed cb d8 31 c0 5d c3 0f 1f 80 00 00 00 00 0f 1f 44 00 00 55 48 89 e5 53 48 8b 5e 38 <48> 8b 7b 50 48 85 ff 74 38 48 8b 87 90 00 00 00 48 8b 97 88 00 [ 9.223183] RIP: sof_widget_unload+0xe/0x90 [snd_sof] RSP: ffffa8cbc06a36a8 [ 9.228835] CR2: 0000000000000050 [ 9.235140] ---[ end trace d9fd6055d01be8a0 ]---
On 03/02/2018 07:38 PM, Pierre-Louis Bossart wrote:
On 03/02/2018 03:21 AM, Xiuli Pan wrote:
From: Pan Xiuli xiuli.pan@linux.intel.com
Add memory window handler for BYT, HSW and BDW.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com
Can you please run checkpatch --strict and resubmit. Nothing personal, it's the same for everyone. I have no appetite for fixing this sort of formatting issues myself. See also comments in the code below.
CHECK: Comparison to NULL could be written "!sdev->info_window" #67: FILE: sound/soc/sof/hw-bdw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #71: FILE: sound/soc/sof/hw-bdw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #79: FILE: sound/soc/sof/hw-bdw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #86: FILE: sound/soc/sof/hw-bdw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #91: FILE: sound/soc/sof/hw-bdw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #96: FILE: sound/soc/sof/hw-bdw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #103: FILE: sound/soc/sof/hw-bdw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #108: FILE: sound/soc/sof/hw-bdw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #117: FILE: sound/soc/sof/hw-bdw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #147: FILE: sound/soc/sof/hw-bdw.c:480: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #189: FILE: sound/soc/sof/hw-byt.c:172: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #193: FILE: sound/soc/sof/hw-byt.c:176: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #201: FILE: sound/soc/sof/hw-byt.c:184: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #208: FILE: sound/soc/sof/hw-byt.c:191: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #213: FILE: sound/soc/sof/hw-byt.c:196: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #218: FILE: sound/soc/sof/hw-byt.c:201: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #225: FILE: sound/soc/sof/hw-byt.c:208: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #230: FILE: sound/soc/sof/hw-byt.c:213: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #239: FILE: sound/soc/sof/hw-byt.c:222: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #269: FILE: sound/soc/sof/hw-byt.c:250: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #303: FILE: sound/soc/sof/hw-hsw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #307: FILE: sound/soc/sof/hw-hsw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #315: FILE: sound/soc/sof/hw-hsw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #322: FILE: sound/soc/sof/hw-hsw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #327: FILE: sound/soc/sof/hw-hsw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #332: FILE: sound/soc/sof/hw-hsw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #339: FILE: sound/soc/sof/hw-hsw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #344: FILE: sound/soc/sof/hw-hsw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #353: FILE: sound/soc/sof/hw-hsw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #382: FILE: sound/soc/sof/hw-hsw.c:479: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
Test with: Mininow max rt5651 GP-MRB nocodec SOF master: 1693b66bb1d804ded975767cc1e5911e6ff9c93c SOF-Tool master: a02abb799405d0e4ad0d6bb46eacf6fbe958c06e https://github.com/plbossart/sound/tree/topic/sof-v4.14: 9513a73b981bc1917705671ec54402a7e21672eb
sound/soc/sof/hw-apl.c | 11 +++++-- sound/soc/sof/hw-bdw.c | 86 ++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-byt.c | 87 +++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-hsw.c | 85 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 233 insertions(+), 36 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index bfbdd915..de0efe1 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1089,9 +1089,13 @@ static int apl_prepare(struct snd_sof_dev *sdev, unsigned int format, static void apl_get_windows(struct snd_sof_dev *sdev) { struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; int i; - u32 inbox_offset = 0, outbox_offset = 0; - u32 inbox_size = 0, outbox_size = 0; if (!sdev->info_window) return; @@ -1135,6 +1139,9 @@ static void apl_get_windows(struct snd_sof_dev *sdev) elem->size, "debug"); break; case SOF_IPC_REGION_STREAM: + stream_offset = + elem->offset + SRAM_WINDOW_OFFSET(elem->id); + stream_size = elem->size; snd_sof_debugfs_create_item(sdev, sdev->bar[APL_DSP_BAR] + elem->offset + diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index bc3141f..5ec0c5b 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = { {"iram", BDW_DSP_BAR, IRAM_OFFSET, BDW_IRAM_SIZE}, {"dram", BDW_DSP_BAR, DRAM_OFFSET, BDW_DRAM_SIZE}, {"shim", BDW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BDW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -379,6 +378,75 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void bdw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
with no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
so here you will init a mailbox of size zero? This looks like an error, why continue with the execution flow.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -394,19 +462,15 @@ static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ bdw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ bdw_get_windows(sdev);
return 0; } diff --git a/sound/soc/sof/hw-byt.c b/sound/soc/sof/hw-byt.c index 209f72a..031cec6 100644 --- a/sound/soc/sof/hw-byt.c +++ b/sound/soc/sof/hw-byt.c @@ -73,7 +73,6 @@ static const struct snd_sof_debugfs_map byt_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; static const struct snd_sof_debugfs_map cht_debugfs[] = { @@ -89,7 +88,6 @@ static const struct snd_sof_debugfs_map cht_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; static void byt_dump(struct snd_sof_dev *sdev, u32 flags) @@ -197,6 +195,75 @@ static void byt_block_read(struct snd_sof_dev *sdev, u32 offset, void *dest, /* * IPC Firmware ready. */ +static void byt_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
same, this looks wrong.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -212,19 +279,15 @@ static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ byt_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ byt_get_windows(sdev);
return 0; } diff --git a/sound/soc/sof/hw-hsw.c b/sound/soc/sof/hw-hsw.c index c6366bb..2f36216 100644 --- a/sound/soc/sof/hw-hsw.c +++ b/sound/soc/sof/hw-hsw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map hsw_debugfs[] = { {"iram", HSW_DSP_BAR, IRAM_OFFSET, HSW_IRAM_SIZE}, {"dram", HSW_DSP_BAR, DRAM_OFFSET, HSW_DRAM_SIZE}, {"shim", HSW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", HSW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -380,6 +379,75 @@ static irqreturn_t hsw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void hsw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
same here, this looks wrong.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -395,18 +463,13 @@ static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ hsw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ hsw_get_windows(sdev); return 0; }
On Fri, 2018-03-02 at 20:16 -0600, Pierre-Louis Bossart wrote:
And on top on my previous comments this patch causes a kernel oops - likely because the matching patches are not yet merged in the firmware master branch. If there is such a dependency between firmware and driver, please make it clear upfront, thanks.
I've now applied the firmware patches that this depends on.
Liam
On 3/5/2018 23:16, Liam Girdwood wrote:
On Fri, 2018-03-02 at 20:16 -0600, Pierre-Louis Bossart wrote:
And on top on my previous comments this patch causes a kernel oops - likely because the matching patches are not yet merged in the firmware master branch. If there is such a dependency between firmware and driver, please make it clear upfront, thanks.
I've now applied the firmware patches that this depends on.
Hi Pierre,
I will send the newest kernel patch ASAP. Please review it and may merge it to make the code base work now.
Thanks Xiuli
Liam
On 3/3/2018 09:38, Pierre-Louis Bossart wrote:
On 03/02/2018 03:21 AM, Xiuli Pan wrote:
From: Pan Xiuli xiuli.pan@linux.intel.com
Add memory window handler for BYT, HSW and BDW.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com
Can you please run checkpatch --strict and resubmit. Nothing personal, it's the same for everyone. I have no appetite for fixing this sort of formatting issues myself. See also comments in the code below.
Sorry about the patch formatting. I just run checkpatch, I will re-check my patches and send them out again.
Thanks Xiuli
CHECK: Comparison to NULL could be written "!sdev->info_window" #67: FILE: sound/soc/sof/hw-bdw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #71: FILE: sound/soc/sof/hw-bdw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #79: FILE: sound/soc/sof/hw-bdw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #86: FILE: sound/soc/sof/hw-bdw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #91: FILE: sound/soc/sof/hw-bdw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #96: FILE: sound/soc/sof/hw-bdw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #103: FILE: sound/soc/sof/hw-bdw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #108: FILE: sound/soc/sof/hw-bdw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #117: FILE: sound/soc/sof/hw-bdw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #147: FILE: sound/soc/sof/hw-bdw.c:480: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #189: FILE: sound/soc/sof/hw-byt.c:172: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #193: FILE: sound/soc/sof/hw-byt.c:176: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #201: FILE: sound/soc/sof/hw-byt.c:184: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #208: FILE: sound/soc/sof/hw-byt.c:191: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #213: FILE: sound/soc/sof/hw-byt.c:196: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #218: FILE: sound/soc/sof/hw-byt.c:201: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #225: FILE: sound/soc/sof/hw-byt.c:208: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #230: FILE: sound/soc/sof/hw-byt.c:213: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #239: FILE: sound/soc/sof/hw-byt.c:222: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #269: FILE: sound/soc/sof/hw-byt.c:250: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #303: FILE: sound/soc/sof/hw-hsw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #307: FILE: sound/soc/sof/hw-hsw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #315: FILE: sound/soc/sof/hw-hsw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #322: FILE: sound/soc/sof/hw-hsw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #327: FILE: sound/soc/sof/hw-hsw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #332: FILE: sound/soc/sof/hw-hsw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #339: FILE: sound/soc/sof/hw-hsw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #344: FILE: sound/soc/sof/hw-hsw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #353: FILE: sound/soc/sof/hw-hsw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #382: FILE: sound/soc/sof/hw-hsw.c:479: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
Test with: Mininow max rt5651 GP-MRB nocodec SOF master: 1693b66bb1d804ded975767cc1e5911e6ff9c93c SOF-Tool master: a02abb799405d0e4ad0d6bb46eacf6fbe958c06e https://github.com/plbossart/sound/tree/topic/sof-v4.14: 9513a73b981bc1917705671ec54402a7e21672eb
sound/soc/sof/hw-apl.c | 11 +++++-- sound/soc/sof/hw-bdw.c | 86 ++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-byt.c | 87 +++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-hsw.c | 85 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 233 insertions(+), 36 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index bfbdd915..de0efe1 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1089,9 +1089,13 @@ static int apl_prepare(struct snd_sof_dev *sdev, unsigned int format, static void apl_get_windows(struct snd_sof_dev *sdev) { struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; int i; - u32 inbox_offset = 0, outbox_offset = 0; - u32 inbox_size = 0, outbox_size = 0; if (!sdev->info_window) return; @@ -1135,6 +1139,9 @@ static void apl_get_windows(struct snd_sof_dev *sdev) elem->size, "debug"); break; case SOF_IPC_REGION_STREAM: + stream_offset = + elem->offset + SRAM_WINDOW_OFFSET(elem->id); + stream_size = elem->size; snd_sof_debugfs_create_item(sdev, sdev->bar[APL_DSP_BAR] + elem->offset + diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index bc3141f..5ec0c5b 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = { {"iram", BDW_DSP_BAR, IRAM_OFFSET, BDW_IRAM_SIZE}, {"dram", BDW_DSP_BAR, DRAM_OFFSET, BDW_DRAM_SIZE}, {"shim", BDW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BDW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -379,6 +378,75 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void bdw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
with no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
so here you will init a mailbox of size zero? This looks like an error, why continue with the execution flow.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -394,19 +462,15 @@ static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ bdw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ bdw_get_windows(sdev);
return 0; } diff --git a/sound/soc/sof/hw-byt.c b/sound/soc/sof/hw-byt.c index 209f72a..031cec6 100644 --- a/sound/soc/sof/hw-byt.c +++ b/sound/soc/sof/hw-byt.c @@ -73,7 +73,6 @@ static const struct snd_sof_debugfs_map byt_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; static const struct snd_sof_debugfs_map cht_debugfs[] = { @@ -89,7 +88,6 @@ static const struct snd_sof_debugfs_map cht_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; static void byt_dump(struct snd_sof_dev *sdev, u32 flags) @@ -197,6 +195,75 @@ static void byt_block_read(struct snd_sof_dev *sdev, u32 offset, void *dest, /* * IPC Firmware ready. */ +static void byt_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
same, this looks wrong.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -212,19 +279,15 @@ static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ byt_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ byt_get_windows(sdev);
return 0; } diff --git a/sound/soc/sof/hw-hsw.c b/sound/soc/sof/hw-hsw.c index c6366bb..2f36216 100644 --- a/sound/soc/sof/hw-hsw.c +++ b/sound/soc/sof/hw-hsw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map hsw_debugfs[] = { {"iram", HSW_DSP_BAR, IRAM_OFFSET, HSW_IRAM_SIZE}, {"dram", HSW_DSP_BAR, DRAM_OFFSET, HSW_DRAM_SIZE}, {"shim", HSW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", HSW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -380,6 +379,75 @@ static irqreturn_t hsw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void hsw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
same here, this looks wrong.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -395,18 +463,13 @@ static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ hsw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ hsw_get_windows(sdev); return 0; }
On 3/3/2018 09:38, Pierre-Louis Bossart wrote:
On 03/02/2018 03:21 AM, Xiuli Pan wrote:
From: Pan Xiuli xiuli.pan@linux.intel.com
Add memory window handler for BYT, HSW and BDW.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com
Can you please run checkpatch --strict and resubmit. Nothing personal, it's the same for everyone. I have no appetite for fixing this sort of formatting issues myself. See also comments in the code below.
CHECK: Comparison to NULL could be written "!sdev->info_window" #67: FILE: sound/soc/sof/hw-bdw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #71: FILE: sound/soc/sof/hw-bdw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #79: FILE: sound/soc/sof/hw-bdw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #86: FILE: sound/soc/sof/hw-bdw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #91: FILE: sound/soc/sof/hw-bdw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #96: FILE: sound/soc/sof/hw-bdw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #103: FILE: sound/soc/sof/hw-bdw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #108: FILE: sound/soc/sof/hw-bdw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #117: FILE: sound/soc/sof/hw-bdw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #147: FILE: sound/soc/sof/hw-bdw.c:480: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #189: FILE: sound/soc/sof/hw-byt.c:172: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #193: FILE: sound/soc/sof/hw-byt.c:176: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #201: FILE: sound/soc/sof/hw-byt.c:184: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #208: FILE: sound/soc/sof/hw-byt.c:191: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #213: FILE: sound/soc/sof/hw-byt.c:196: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #218: FILE: sound/soc/sof/hw-byt.c:201: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #225: FILE: sound/soc/sof/hw-byt.c:208: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #230: FILE: sound/soc/sof/hw-byt.c:213: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #239: FILE: sound/soc/sof/hw-byt.c:222: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #269: FILE: sound/soc/sof/hw-byt.c:250: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #303: FILE: sound/soc/sof/hw-hsw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #307: FILE: sound/soc/sof/hw-hsw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #315: FILE: sound/soc/sof/hw-hsw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #322: FILE: sound/soc/sof/hw-hsw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #327: FILE: sound/soc/sof/hw-hsw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #332: FILE: sound/soc/sof/hw-hsw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #339: FILE: sound/soc/sof/hw-hsw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #344: FILE: sound/soc/sof/hw-hsw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #353: FILE: sound/soc/sof/hw-hsw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #382: FILE: sound/soc/sof/hw-hsw.c:479: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
Test with: Mininow max rt5651 GP-MRB nocodec SOF master: 1693b66bb1d804ded975767cc1e5911e6ff9c93c SOF-Tool master: a02abb799405d0e4ad0d6bb46eacf6fbe958c06e https://github.com/plbossart/sound/tree/topic/sof-v4.14: 9513a73b981bc1917705671ec54402a7e21672eb
sound/soc/sof/hw-apl.c | 11 +++++-- sound/soc/sof/hw-bdw.c | 86 ++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-byt.c | 87 +++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-hsw.c | 85 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 233 insertions(+), 36 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index bfbdd915..de0efe1 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1089,9 +1089,13 @@ static int apl_prepare(struct snd_sof_dev *sdev, unsigned int format, static void apl_get_windows(struct snd_sof_dev *sdev) { struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; int i; - u32 inbox_offset = 0, outbox_offset = 0; - u32 inbox_size = 0, outbox_size = 0; if (!sdev->info_window) return; @@ -1135,6 +1139,9 @@ static void apl_get_windows(struct snd_sof_dev *sdev) elem->size, "debug"); break; case SOF_IPC_REGION_STREAM: + stream_offset = + elem->offset + SRAM_WINDOW_OFFSET(elem->id); + stream_size = elem->size; snd_sof_debugfs_create_item(sdev, sdev->bar[APL_DSP_BAR] + elem->offset + diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index bc3141f..5ec0c5b 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = { {"iram", BDW_DSP_BAR, IRAM_OFFSET, BDW_IRAM_SIZE}, {"dram", BDW_DSP_BAR, DRAM_OFFSET, BDW_DRAM_SIZE}, {"shim", BDW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BDW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -379,6 +378,75 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void bdw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
with no error?
Sorry about this, copy the function base from the hw-apl.c. The function has no return type. What should we handle the error here.
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
so here you will init a mailbox of size zero? This looks like an error, why continue with the execution flow.
What do you mean size zero here? This is a dynamic parse for the memory window info passed from the DSP. We may change the memory window setting in the FW and here we just get and create the memory window what the DSP send to the host.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -394,19 +462,15 @@ static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ bdw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ bdw_get_windows(sdev);
return 0; } diff --git a/sound/soc/sof/hw-byt.c b/sound/soc/sof/hw-byt.c index 209f72a..031cec6 100644 --- a/sound/soc/sof/hw-byt.c +++ b/sound/soc/sof/hw-byt.c @@ -73,7 +73,6 @@ static const struct snd_sof_debugfs_map byt_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; static const struct snd_sof_debugfs_map cht_debugfs[] = { @@ -89,7 +88,6 @@ static const struct snd_sof_debugfs_map cht_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; static void byt_dump(struct snd_sof_dev *sdev, u32 flags) @@ -197,6 +195,75 @@ static void byt_block_read(struct snd_sof_dev *sdev, u32 offset, void *dest, /* * IPC Firmware ready. */ +static void byt_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
same, this looks wrong.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -212,19 +279,15 @@ static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ byt_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ byt_get_windows(sdev);
return 0; } diff --git a/sound/soc/sof/hw-hsw.c b/sound/soc/sof/hw-hsw.c index c6366bb..2f36216 100644 --- a/sound/soc/sof/hw-hsw.c +++ b/sound/soc/sof/hw-hsw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map hsw_debugfs[] = { {"iram", HSW_DSP_BAR, IRAM_OFFSET, HSW_IRAM_SIZE}, {"dram", HSW_DSP_BAR, DRAM_OFFSET, HSW_DRAM_SIZE}, {"shim", HSW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", HSW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -380,6 +379,75 @@ static irqreturn_t hsw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void hsw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
same here, this looks wrong.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -395,18 +463,13 @@ static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ hsw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ hsw_get_windows(sdev); return 0; }
sound/soc/sof/hw-apl.c | 11 +++++-- sound/soc/sof/hw-bdw.c | 86 ++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-byt.c | 87 +++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-hsw.c | 85 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 233 insertions(+), 36 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index bfbdd915..de0efe1 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1089,9 +1089,13 @@ static int apl_prepare(struct snd_sof_dev *sdev, unsigned int format, static void apl_get_windows(struct snd_sof_dev *sdev) { struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; int i; - u32 inbox_offset = 0, outbox_offset = 0; - u32 inbox_size = 0, outbox_size = 0; if (!sdev->info_window) return; @@ -1135,6 +1139,9 @@ static void apl_get_windows(struct snd_sof_dev *sdev) elem->size, "debug"); break; case SOF_IPC_REGION_STREAM: + stream_offset = + elem->offset + SRAM_WINDOW_OFFSET(elem->id); + stream_size = elem->size; snd_sof_debugfs_create_item(sdev, sdev->bar[APL_DSP_BAR] + elem->offset + diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index bc3141f..5ec0c5b 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = { {"iram", BDW_DSP_BAR, IRAM_OFFSET, BDW_IRAM_SIZE}, {"dram", BDW_DSP_BAR, DRAM_OFFSET, BDW_DRAM_SIZE}, {"shim", BDW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BDW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -379,6 +378,75 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void bdw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
with no error?
Sorry about this, copy the function base from the hw-apl.c. The function has no return type. What should we handle the error here.
if there is an error then there should be a way to inform the caller than something went wrong - or add a dev_err.
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
so here you will init a mailbox of size zero? This looks like an error, why continue with the execution flow.
What do you mean size zero here? This is a dynamic parse for the memory window info passed from the DSP. We may change the memory window setting in the FW and here we just get and create the memory window what the DSP send to the host.
in the default case, all local variables remain set to zero, so that case is likely a major issue where you would need to return an error, not continue with a simple break.
the same comments for the other platforms apply, error cases are not handled.
On 3/5/2018 22:54, Pierre-Louis Bossart wrote:
sound/soc/sof/hw-apl.c | 11 +++++-- sound/soc/sof/hw-bdw.c | 86 ++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-byt.c | 87 +++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-hsw.c | 85 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 233 insertions(+), 36 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index bfbdd915..de0efe1 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1089,9 +1089,13 @@ static int apl_prepare(struct snd_sof_dev *sdev, unsigned int format, static void apl_get_windows(struct snd_sof_dev *sdev) { struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; int i; - u32 inbox_offset = 0, outbox_offset = 0; - u32 inbox_size = 0, outbox_size = 0; if (!sdev->info_window) return; @@ -1135,6 +1139,9 @@ static void apl_get_windows(struct snd_sof_dev *sdev) elem->size, "debug"); break; case SOF_IPC_REGION_STREAM: + stream_offset = + elem->offset + SRAM_WINDOW_OFFSET(elem->id); + stream_size = elem->size; snd_sof_debugfs_create_item(sdev, sdev->bar[APL_DSP_BAR] + elem->offset + diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index bc3141f..5ec0c5b 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = { {"iram", BDW_DSP_BAR, IRAM_OFFSET, BDW_IRAM_SIZE}, {"dram", BDW_DSP_BAR, DRAM_OFFSET, BDW_DRAM_SIZE}, {"shim", BDW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BDW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -379,6 +378,75 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void bdw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
with no error?
Sorry about this, copy the function base from the hw-apl.c. The function has no return type. What should we handle the error here.
if there is an error then there should be a way to inform the caller than something went wrong - or add a dev_err.
I will add a dev_err here. This function is designed to parse the message from FW. It expected that DSP give the right things, but may need to debug if anything goes wrong.
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
so here you will init a mailbox of size zero? This looks like an error, why continue with the execution flow.
What do you mean size zero here? This is a dynamic parse for the memory window info passed from the DSP. We may change the memory window setting in the FW and here we just get and create the memory window what the DSP send to the host.
in the default case, all local variables remain set to zero, so that case is likely a major issue where you would need to return an error, not continue with a simple break.
the same comments for the other platforms apply, error cases are not handled.
Will add a dev_err here too to inform that we got some illegal memory window from the DSP. Will need to check the FW if we have send something wrong. Will also make some change to the hw-apl.c function.
May have some refine about the BAR value and memory window hard code, then we may merge these functions into one function.
Thanks Xiuli
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On 2018年03月03日 09:38, Pierre-Louis Bossart wrote:
On 03/02/2018 03:21 AM, Xiuli Pan wrote:
From: Pan Xiuli xiuli.pan@linux.intel.com
Add memory window handler for BYT, HSW and BDW.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com
Can you please run checkpatch --strict and resubmit. Nothing personal, it's the same for everyone. I have no appetite for fixing this sort of formatting issues myself. See also comments in the code below.
Hi Pierre, does it make sense to enable/hook this like what we do in SOF repo?
Thanks, ~Keyon
CHECK: Comparison to NULL could be written "!sdev->info_window" #67: FILE: sound/soc/sof/hw-bdw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #71: FILE: sound/soc/sof/hw-bdw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #79: FILE: sound/soc/sof/hw-bdw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #86: FILE: sound/soc/sof/hw-bdw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #91: FILE: sound/soc/sof/hw-bdw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #96: FILE: sound/soc/sof/hw-bdw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #103: FILE: sound/soc/sof/hw-bdw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #108: FILE: sound/soc/sof/hw-bdw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #117: FILE: sound/soc/sof/hw-bdw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #147: FILE: sound/soc/sof/hw-bdw.c:480: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #189: FILE: sound/soc/sof/hw-byt.c:172: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #193: FILE: sound/soc/sof/hw-byt.c:176: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #201: FILE: sound/soc/sof/hw-byt.c:184: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #208: FILE: sound/soc/sof/hw-byt.c:191: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #213: FILE: sound/soc/sof/hw-byt.c:196: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #218: FILE: sound/soc/sof/hw-byt.c:201: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #225: FILE: sound/soc/sof/hw-byt.c:208: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #230: FILE: sound/soc/sof/hw-byt.c:213: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #239: FILE: sound/soc/sof/hw-byt.c:222: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #269: FILE: sound/soc/sof/hw-byt.c:250: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
CHECK: Comparison to NULL could be written "!sdev->info_window" #303: FILE: sound/soc/sof/hw-hsw.c:402: + if (sdev->info_window == NULL)
CHECK: Blank lines aren't necessary after an open brace '{' #307: FILE: sound/soc/sof/hw-hsw.c:406: + for (i = 0; i < sdev->info_window->num_windows; i++) {
CHECK: Alignment should match open parenthesis #315: FILE: sound/soc/sof/hw-hsw.c:414: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset,
CHECK: Alignment should match open parenthesis #322: FILE: sound/soc/sof/hw-hsw.c:421: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset,
CHECK: Alignment should match open parenthesis #327: FILE: sound/soc/sof/hw-hsw.c:426: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #332: FILE: sound/soc/sof/hw-hsw.c:431: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #339: FILE: sound/soc/sof/hw-hsw.c:438: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset,
CHECK: Alignment should match open parenthesis #344: FILE: sound/soc/sof/hw-hsw.c:443: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset +
CHECK: Alignment should match open parenthesis #353: FILE: sound/soc/sof/hw-hsw.c:452: + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
CHECK: Alignment should match open parenthesis #382: FILE: sound/soc/sof/hw-hsw.c:479: + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
Test with: Mininow max rt5651 GP-MRB nocodec SOF master: 1693b66bb1d804ded975767cc1e5911e6ff9c93c SOF-Tool master: a02abb799405d0e4ad0d6bb46eacf6fbe958c06e https://github.com/plbossart/sound/tree/topic/sof-v4.14: 9513a73b981bc1917705671ec54402a7e21672eb
sound/soc/sof/hw-apl.c | 11 +++++-- sound/soc/sof/hw-bdw.c | 86 ++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-byt.c | 87 +++++++++++++++++++++++++++++++++++++++++++------- sound/soc/sof/hw-hsw.c | 85 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 233 insertions(+), 36 deletions(-)
diff --git a/sound/soc/sof/hw-apl.c b/sound/soc/sof/hw-apl.c index bfbdd915..de0efe1 100644 --- a/sound/soc/sof/hw-apl.c +++ b/sound/soc/sof/hw-apl.c @@ -1089,9 +1089,13 @@ static int apl_prepare(struct snd_sof_dev *sdev, unsigned int format, static void apl_get_windows(struct snd_sof_dev *sdev) { struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; int i; - u32 inbox_offset = 0, outbox_offset = 0; - u32 inbox_size = 0, outbox_size = 0; if (!sdev->info_window) return; @@ -1135,6 +1139,9 @@ static void apl_get_windows(struct snd_sof_dev *sdev) elem->size, "debug"); break; case SOF_IPC_REGION_STREAM: + stream_offset = + elem->offset + SRAM_WINDOW_OFFSET(elem->id); + stream_size = elem->size; snd_sof_debugfs_create_item(sdev, sdev->bar[APL_DSP_BAR] + elem->offset + diff --git a/sound/soc/sof/hw-bdw.c b/sound/soc/sof/hw-bdw.c index bc3141f..5ec0c5b 100644 --- a/sound/soc/sof/hw-bdw.c +++ b/sound/soc/sof/hw-bdw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = { {"iram", BDW_DSP_BAR, IRAM_OFFSET, BDW_IRAM_SIZE}, {"dram", BDW_DSP_BAR, DRAM_OFFSET, BDW_DRAM_SIZE}, {"shim", BDW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BDW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -379,6 +378,75 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void bdw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
with no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BDW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
so here you will init a mailbox of size zero? This looks like an error, why continue with the execution flow.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -394,19 +462,15 @@ static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ bdw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ bdw_get_windows(sdev);
return 0; } diff --git a/sound/soc/sof/hw-byt.c b/sound/soc/sof/hw-byt.c index 209f72a..031cec6 100644 --- a/sound/soc/sof/hw-byt.c +++ b/sound/soc/sof/hw-byt.c @@ -73,7 +73,6 @@ static const struct snd_sof_debugfs_map byt_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; static const struct snd_sof_debugfs_map cht_debugfs[] = { @@ -89,7 +88,6 @@ static const struct snd_sof_debugfs_map cht_debugfs[] = { {"iram", BYT_DSP_BAR, IRAM_OFFSET, IRAM_SIZE}, {"dram", BYT_DSP_BAR, DRAM_OFFSET, DRAM_SIZE}, {"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", BYT_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; static void byt_dump(struct snd_sof_dev *sdev, u32 flags) @@ -197,6 +195,75 @@ static void byt_block_read(struct snd_sof_dev *sdev, u32 offset, void *dest, /* * IPC Firmware ready. */ +static void byt_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[BYT_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
same, this looks wrong.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -212,19 +279,15 @@ static int byt_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ byt_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ byt_get_windows(sdev);
return 0; } diff --git a/sound/soc/sof/hw-hsw.c b/sound/soc/sof/hw-hsw.c index c6366bb..2f36216 100644 --- a/sound/soc/sof/hw-hsw.c +++ b/sound/soc/sof/hw-hsw.c @@ -63,7 +63,6 @@ static const struct snd_sof_debugfs_map hsw_debugfs[] = { {"iram", HSW_DSP_BAR, IRAM_OFFSET, HSW_IRAM_SIZE}, {"dram", HSW_DSP_BAR, DRAM_OFFSET, HSW_DRAM_SIZE}, {"shim", HSW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE}, - {"mbox", HSW_DSP_BAR, MBOX_OFFSET, MBOX_SIZE}, }; /* @@ -380,6 +379,75 @@ static irqreturn_t hsw_irq_thread(int irq, void *context) /* * IPC Firmware ready. */ +static void hsw_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i;
+ if (sdev->info_window == NULL) + return;
no error?
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+ switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + inbox_offset, + elem->size, "inbox"); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + outbox_offset, + elem->size, "outbox"); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "etrace"); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "debug"); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + stream_offset, + elem->size, "stream"); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_create_item(sdev, + sdev->bar[HSW_DSP_BAR] + elem->offset + + MBOX_OFFSET, elem->size, "regs"); + break; + default: + break;
same here, this looks wrong.
+ } + }
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); +}
static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) { struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; @@ -395,18 +463,13 @@ static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) /* copy data from the DSP FW ready offset */ hsw_block_read(sdev, offset, fw_ready, sizeof(*fw_ready)); - snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, - fw_ready->dspbox_size, - fw_ready->hostbox_offset, - fw_ready->hostbox_size);
- dev_dbg(sdev->dev, " mailbox DSP initiated 0x%x - size 0x%x\n", - fw_ready->dspbox_offset, fw_ready->dspbox_size); - dev_dbg(sdev->dev, " mailbox Host initiated 0x%x - size 0x%x\n", - fw_ready->hostbox_offset, fw_ready->hostbox_size);
dev_info(sdev->dev, " Firmware info: version %d:%d-%s build %d on %s:%s\n", v->major, v->minor, v->tag, v->build, v->date, v->time); + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, + MBOX_OFFSET + sizeof(struct sof_ipc_fw_ready));
+ hsw_get_windows(sdev); return 0; }
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
participants (5)
-
Keyon Jie -
Liam Girdwood -
Pan, Xiuli -
Pierre-Louis Bossart -
Xiuli Pan