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

[Daniel Baluta]

On Wed, Jan 23, 2019 at 6:16 PM Liam Girdwood
<liam.r.girdwood at linux.intel.com> wrote:
>
> On Wed, 2019-01-23 at 16:42 +0200, Daniel Baluta wrote:
> > Please s/Where/What/ in the title. :))
> >
> > On Wed, Jan 23, 2019 at 4:41 PM Daniel Baluta <daniel.baluta at gmail.com> wrote:
> > > Hi Pierre/Liam,
> > >
> > > In src/include/sof/ipc.h there is:
> > >
> > > struct ipc {
> > >      uint32_t host_msg;»     »       /* current message from host */
> > > //
> > > }
> > >
> > > This keeps the message received from IA core. Anyhow, DSP core
> > > doesn't seem to use it at all.
> > >
>
> Yeah, this is legacy that could be removed.
>
> > > Except maybe for some logging:
> > >
> > > src/drivers/intel/baytrail/ipc.c:                       _ipc->host_msg =
> > > msg;
> > > src/drivers/intel/baytrail/ipc.c:       trace_ipc("ipc: msg rx ->
> > > 0x%x", ipc->host_msg);
> > > src/drivers/intel/cavs/ipc.c:                   _ipc->host_msg = msg;
> > > src/drivers/intel/cavs/ipc.c:   trace_ipc("ipc: msg rx -> 0x%x", ipc-
> > > >host_msg);
> > > src/drivers/intel/cavs/sue-ipc.c:       trace_ipc("ipc: msg rx ->
> > > 0x%x", ipc->host_msg);
> > > src/drivers/intel/haswell/ipc.c:                        _ipc->host_msg =
> > > msg;
> > > src/drivers/intel/haswell/ipc.c:        trace_ipc("ipc: msg rx ->
> > > 0x%x", ipc->host_msg);
> > > src/include/sof/ipc.h:  uint32_t host_msg;              /* current
> > > message from host */
> > >
> > > I am asking this because initially I thought the communication between
> > > IA core and DSP involves:
> > >
> > > - sending cmd using shim layer
> > > - sending actual data using mailbox.
>
> Yep, on the Intel MMIO based architecture we can think of the SHIM as the
> doorbell and shared memory as the mailbox. The SHIM doorbells can assert IRQs on
> host and DSP (one doorbell for host -> dsp and the other for DSP -> host)
>
> > >
> > > 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.
>
> > > If not, I think it is best to remove it because it is confusing.
>
> Feel free to send PR and remove :)

Will do.

>
> > >
> > > The same happens when DSP tries to send something to IA.
> > >
> > > For example it writes msg->header. But this is never used on the other side.
> > >
> > > ipc_platform_send_msg
> > >
> > > »       /* now interrupt host to tell it we have message sent */
> > > »       shim_write(SHIM_IPCDL, msg->header);
> > > »       shim_write(SHIM_IPCDH, SHIM_IPCDH_BUSY);
> > >
> > > Also, would be easier for me on the ARM side to only send info via mailbox
> > > and
> > > notification via what you call shim layer.
>
> Also makes it easier for SPI and virtio too.

It looks like the reply from DSP sent via shim register it used on the
IA core side, though.

On DSP:

void ipc_platform_send_msg(struct ipc *ipc)
»       /* now interrupt host to tell it we have message sent */
»       shim_write(SHIM_IPCDL, msg->header);
»       shim_write(SHIM_IPCDH, SHIM_IPCDH_BUSY);

On IA core:

byt_irq_thread
    ipcx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCX);
    snd_sof_ipc_reply(sdev, ipcx)

So, in order to find the initial message for which this reply is intended
we transmit the information inside the SHIM_IPCX register.

Will try to see if we can move the id inside the mailbox area, and only
keep the SHIM as a true doorbell.

Thanks a lot for explanations Liam.

Daniel.