[Sound-open-firmware] SOF: Where is the purpose of host_msg ?

[Daniel Baluta]

On Wed, Jan 23, 2019 at 11:10 PM Pierre-Louis Bossart
<pierre-louis.bossart at linux.intel.com> wrote:
>
>
> 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 at 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?

Yes, would be nice to hear from Liam the history of these changes.

It looks to me that at some point the code switched from using SHIM
to mailbox but code cleanup was never finished.

On the TX side this switched looks complete (modulo the cleanup).

But on the RX side there is still SHIM registers used for getting the
replies :(.

Thanks,
Daniel.