On 1/23/19 12:50 PM, Daniel Baluta wrote:
On Wed, Jan 23, 2019 at 8:32 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
But it seems that all the information is exchanged via the mailbox memory. So, going back to my question will host_msg be used in the future?
Yes, this was changed as mailbox was a more architecture neutral method of IPC i.e. it did not depend on passing metadata via special registers.
we still have a case for Baytrail (also reported by Daniel) where the IPC seems to mix commands and doorbell status:
static int byt_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) { u64 cmd = msg->header;
/* send the message */ sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data, msg->msg_size); snd_sof_dsp_write64(sdev, BYT_DSP_BAR, SHIM_IPCX, cmd | SHIM_BYT_IPCX_BUSY); <<< WTH? return 0;There is also:
- hda_dsp_ipc_send_msg
- cnl_ipc_send_msg
It looks like cmd could be removed from all of them. Will send a patch later if anyone agrees that ORing cmd inside the IPCX register is not needed.
I wonder if this doesn't hide a much larger issue with even more code refactoring needed.
see e.g. for PCM, we do this:
/* set IPC PCM parameters */ pcm.hdr.size = sizeof(pcm); pcm.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_PCM_PARAMS; [...] /* send IPC to the DSP */ ret = sof_ipc_tx_message(sdev->ipc, pcm.hdr.cmd, &pcm, sizeof(pcm), &ipc_params_reply, sizeof(ipc_params_reply));
I just don't get why this second "header" argument is needed and what it is used for in the ipc.c file. There is all sorts of code in there that deals with headers, message_id, etc, and all of that looks like needless complexity and layers of crud that needs to do.
If we really want to have a simpler communication scheme everything should be in-band and part of the same data structure, without duplication or shortcuts.
Liam?
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware