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