Skip to content

IPC for upstream #695

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
plbossart merged 9 commits into from
Mar 14, 2019
Merged

IPC for upstream #695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9b5ecfe
ASoC: SOF: (cosmetic) reduce nesting depth
lyakh Mar 1, 2019
92ca10f
ASoC: SOF: remove the RX work
lyakh Mar 3, 2019
6f7c3fe
ASoC: SOF: removed the unused .wait_txq field
lyakh Mar 3, 2019
5f811e1
ASoC: SOF: use a mutex for IPC TX serialisation
lyakh Mar 4, 2019
fd3c962
ASoC: SOF: fix a theoretical race
lyakh Mar 8, 2019
0f9ceb9
ASoC: SOF: remove IPC TX scheduling
lyakh Mar 7, 2019
6ad5661
ASoC: SOF: remove IPC message lists
lyakh Mar 8, 2019
b8bf22b
ASoC: SOF: protect against buffer overflow
lyakh Mar 8, 2019
20d02b3
ASoC: SOF: remove the "is_ipc_ready" API
lyakh Mar 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions sound/soc/sof/hw-spi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,6 @@ static irqreturn_t spi_irq_thread(int irq __maybe_unused, void *context __maybe_
return IRQ_HANDLED;
}

static int spi_is_ipc_ready(struct snd_sof_dev *sdev __maybe_unused)
{
// use local variable to store DSP command state. either DSP is ready
// for new cmd or still processing current cmd.

return 1;
}

static int spi_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
{
/* send the message */
Expand Down Expand Up @@ -300,7 +292,6 @@ const struct snd_sof_dsp_ops snd_sof_spi_ops = {
.send_msg = spi_send_msg,
.get_reply = spi_get_reply,
.fw_ready = spi_fw_ready,
.is_ipc_ready = spi_is_ipc_ready,
.cmd_done = spi_cmd_done,

/* debug */
Expand Down
1 change: 0 additions & 1 deletion sound/soc/sof/intel/apl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ const struct snd_sof_dsp_ops sof_apl_ops = {
.send_msg = hda_dsp_ipc_send_msg,
.get_reply = hda_dsp_ipc_get_reply,
.fw_ready = hda_dsp_ipc_fw_ready,
.is_ipc_ready = hda_dsp_is_ipc_ready,
.cmd_done = hda_dsp_ipc_cmd_done,

/* debug */
Expand Down
12 changes: 0 additions & 12 deletions sound/soc/sof/intel/bdw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,17 +465,6 @@ static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
* IPC Mailbox IO
*/

static int bdw_is_ipc_ready(struct snd_sof_dev *sdev)
{
u32 val;

val = snd_sof_dsp_read(sdev, BDW_DSP_BAR, SHIM_IPCX);
if ((val & SHIM_IPCX_BUSY) || (val & SHIM_IPCX_DONE))
return 0;

return 1;
}

static int bdw_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
{
/* send the message */
Expand Down Expand Up @@ -677,7 +666,6 @@ const struct snd_sof_dsp_ops sof_bdw_ops = {
.send_msg = bdw_send_msg,
.get_reply = bdw_get_reply,
.fw_ready = bdw_fw_ready,
.is_ipc_ready = bdw_is_ipc_ready,
.cmd_done = bdw_cmd_done,

/* debug */
Expand Down
14 changes: 0 additions & 14 deletions sound/soc/sof/intel/byt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,17 +360,6 @@ static irqreturn_t byt_irq_thread(int irq, void *context)
return IRQ_HANDLED;
}

static int byt_is_ipc_ready(struct snd_sof_dev *sdev)
{
u64 ipcx;

ipcx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCX);
if ((ipcx & SHIM_BYT_IPCX_BUSY) || (ipcx & SHIM_BYT_IPCX_DONE))
return 0;

return 1;
}

@RanderWang RanderWang Mar 7, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ipcx & SHIM_BYT_IPCX_DONE is removed, so cmd_done function should be refined because SHIM_BYT_IPCX_DONE is set for is_ipc_ready. And please refine other platforms.

@RanderWang RanderWang Mar 7, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is my advice. You could also refer to this branch https://github.com/thesofproject/linux/tree/archive/topic/sof-dev-20180619/sound/soc/sof/intel to get the original code. We added some code to fix IPC bugs, now they are not needed. Please only change the code for host sending msg to FW, not FW to host now.

-       if (ipcx & SHIM_BYT_IPCX_DONE &&
-           !(imrx & SHIM_IMRX_DONE)) {
-               /* Mask Done interrupt before first */
-               snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR,
-                                                  SHIM_IMRX,
-                                                  SHIM_IMRX_DONE,
-                                                  SHIM_IMRX_DONE);
-               /*
-                * handle immediate reply from DSP core. If the msg is
-                * found, set done bit in cmd_done which is called at the
-                * end of message processing function, else set it here
-                * because the done bit can't be set in cmd_done function
-                * which is triggered by msg
-                */
-               if (snd_sof_ipc_reply(sdev, ipcx))
-                       byt_cmd_done(sdev, SOF_IPC_DSP_REPLY);
+       if (ipcx & SHIM_BYT_IPCX_DONE) {
+
+               snd_sof_ipc_reply(sdev, ipcx);
+
+                               /* clear DONE bit - tell DSP we have completed */
+               snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IPCX,
+                                                  SHIM_BYT_IPCX_DONE, 0);
+
+           /* unmask Done interrupt */
+               snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IMRX,
+                                                  SHIM_IMRX_DONE, 0)
        }
 
        /* new message from DSP */
@@ -427,14 +422,6 @@ static int byt_cmd_done(struct snd_sof_dev *sdev, int dir)
                /* unmask busy interrupt */
                snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IMRX,
                                                   SHIM_IMRX_BUSY, 0);
-       } else {
-               /* clear DONE bit - tell DSP we have completed */
-               snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IPCX,
-                                                  SHIM_BYT_IPCX_DONE, 0);
-
-               /* unmask Done interrupt */
-               snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IMRX,
-                                                  SHIM_IMRX_DONE, 0);
        }

@lyakh lyakh Mar 7, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hm, I think there's a bug in my patch set. Looks like there are indeed cases when snd_sof_ipc_reply() has to return a non-zero value, specifically when no message is being transmitted. I'll fix that and will think about your comments. Thanks!

@lyakh lyakh Mar 7, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Concerning your comment. It seems to make sense to me, but I'd postpone it to a separate patch or a patch set. With this patch set I tried to change the actual control flow as little as possible. This would be a significant change. So, unless it is really tightly coupled to this patch set, I'd rather postpone it to a later PR.

static int byt_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
{
u64 cmd = msg->header;
Expand Down Expand Up @@ -616,7 +605,6 @@ const struct snd_sof_dsp_ops sof_tng_ops = {
.send_msg = byt_send_msg,
.get_reply = byt_get_reply,
.fw_ready = byt_fw_ready,
.is_ipc_ready = byt_is_ipc_ready,
.cmd_done = byt_cmd_done,

/* debug */
Expand Down Expand Up @@ -778,7 +766,6 @@ const struct snd_sof_dsp_ops sof_byt_ops = {
.send_msg = byt_send_msg,
.get_reply = byt_get_reply,
.fw_ready = byt_fw_ready,
.is_ipc_ready = byt_is_ipc_ready,
.cmd_done = byt_cmd_done,

/* debug */
Expand Down Expand Up @@ -835,7 +822,6 @@ const struct snd_sof_dsp_ops sof_cht_ops = {
.send_msg = byt_send_msg,
.get_reply = byt_get_reply,
.fw_ready = byt_fw_ready,
.is_ipc_ready = byt_is_ipc_ready,
.cmd_done = byt_cmd_done,

/* debug */
Expand Down
14 changes: 0 additions & 14 deletions sound/soc/sof/intel/cnl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,19 +153,6 @@ static int cnl_ipc_cmd_done(struct snd_sof_dev *sdev, int dir)
return 0;
}

static int cnl_is_ipc_ready(struct snd_sof_dev *sdev)
{
u32 busy, done;

busy = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDR);
done = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDA);
if ((busy & CNL_DSP_REG_HIPCIDR_BUSY) ||
(done & CNL_DSP_REG_HIPCIDA_DONE))
return 0;

return 1;
}

static int cnl_ipc_send_msg(struct snd_sof_dev *sdev,
struct snd_sof_ipc_msg *msg)
{
Expand Down Expand Up @@ -208,7 +195,6 @@ const struct snd_sof_dsp_ops sof_cnl_ops = {
.send_msg = cnl_ipc_send_msg,
.get_reply = hda_dsp_ipc_get_reply,
.fw_ready = hda_dsp_ipc_fw_ready,
.is_ipc_ready = cnl_is_ipc_ready,
.cmd_done = cnl_ipc_cmd_done,

/* debug */
Expand Down
16 changes: 1 addition & 15 deletions sound/soc/sof/intel/hda-ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,6 @@ int hda_dsp_ipc_cmd_done(struct snd_sof_dev *sdev, int dir)
return 0;
}

int hda_dsp_is_ipc_ready(struct snd_sof_dev *sdev)
{
u32 busy, done;

/* is DSP ready for next IPC command */
busy = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_HIPCI);
done = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_HIPCIE);
if ((busy & HDA_DSP_REG_HIPCI_BUSY) ||
(done & HDA_DSP_REG_HIPCIE_DONE))
return 0;

return 1;
}

int hda_dsp_ipc_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
{
u32 cmd = msg->header;
Expand All @@ -90,7 +76,7 @@ int hda_dsp_ipc_get_reply(struct snd_sof_dev *sdev,
int ret = 0;
u32 size;

hdr = (struct sof_ipc_cmd_hdr *)msg->msg_data;
hdr = msg->msg_data;
if (hdr->cmd == (SOF_IPC_GLB_PM_MSG | SOF_IPC_PM_CTX_SAVE)) {
/*
* memory windows are powered off before sending IPC reply,
Expand Down
1 change: 0 additions & 1 deletion sound/soc/sof/intel/hda.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,6 @@ int hda_dsp_stream_spib_config(struct snd_sof_dev *sdev,
/*
* DSP IPC Operations.
*/
int hda_dsp_is_ipc_ready(struct snd_sof_dev *sdev);
int hda_dsp_ipc_send_msg(struct snd_sof_dev *sdev,
struct snd_sof_ipc_msg *msg);
int hda_dsp_ipc_get_reply(struct snd_sof_dev *sdev,
Expand Down
12 changes: 0 additions & 12 deletions sound/soc/sof/intel/hsw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,17 +466,6 @@ static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
* IPC Mailbox IO
*/

static int hsw_is_ipc_ready(struct snd_sof_dev *sdev)
{
u32 val;

val = snd_sof_dsp_read(sdev, HSW_DSP_BAR, SHIM_IPCX);
if ((val & SHIM_IPCX_BUSY) || (val & SHIM_IPCX_DONE))
return 0;

return 1;
}

static int hsw_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
{
/* send the message */
Expand Down Expand Up @@ -677,7 +666,6 @@ const struct snd_sof_dsp_ops sof_hsw_ops = {
.send_msg = hsw_send_msg,
.get_reply = hsw_get_reply,
.fw_ready = hsw_fw_ready,
.is_ipc_ready = hsw_is_ipc_ready,
.cmd_done = hsw_cmd_done,

/* debug */
Expand Down
1 change: 0 additions & 1 deletion sound/soc/sof/intel/skl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ const struct snd_sof_dsp_ops sof_skl_ops = {
.send_msg = hda_dsp_ipc_send_msg,
.get_reply = hda_dsp_ipc_get_reply,
.fw_ready = hda_dsp_ipc_fw_ready,
.is_ipc_ready = hda_dsp_is_ipc_ready,
.cmd_done = hda_dsp_ipc_cmd_done,

/* debug */
Expand Down
Loading