Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver

[Jassi Brar]

On Mon, Dec 7, 2020 at 12:43 PM Daniele Alessandrelli
<daniele.alessandrelli@xxxxxxxxxxxxxxx> wrote:
>
> Hi Rob,
>
> Thanks for the feedback.
>
> On Mon, 2020-12-07 at 10:01 -0600, Rob Herring wrote:
> > On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgross@xxxxxxxxxxxxxxx wrote:
> > > From: Daniele Alessandrelli <daniele.alessandrelli@xxxxxxxxx>
> > >
> > > Add DT binding documentation for the Intel Keem Bay IPC driver, which
> > > enables communication between the Computing Sub-System (CSS) and the
> > > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
> > > Bay.
> > >
>
> [cut]
>
> > > +
> > > +description:
> > > +  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
> > > +  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
> > > +  Keem Bay.
> >
> > Sounds like a mailbox.
>
> We did consider using the mailbox framework, but eventually decided
> against it; mainly because of the following two reasons:
>
> 1. The channel concept in the Mailbox framework is different than the
>    channel concept in Keem Bay IPC:
>
>    a. My understanding is that Mailbox channels are meant to be SW
>       representation of physical HW channels, while in Keem Bay IPC
>       channels are software abstractions to achieve communication
>       multiplexing over a single HW link
>
In mailbox api, that would be a physical channel shared between various clients.

>    b. Additionally, Keem Bay IPC has two different classes of channels
>       (high-speed channels and general-purpose channels) that need to
>       access the same HW link with different priorities.
>
If the priorities are hard (programmed into some register), you could
do that via dt during channel population.
If they are soft, that would be handled in the shared channel implementation.

> 2. The blocking / non-blocking TX behavior of mailbox channels is
>    defined at channel creation time (by the tx_block value of the
>    mailbox client passed to mbox_request_channel();
>
No, that is checked at mbox_send_message()

> my understanding
>    is that the tx_block value cannot be modified after the channel is
>    created),
>
Again no. If you don't queue more than one message at any time you can
change it between transfers. To be safe you can always change it
between channel release - request calls.

>  while in Keem Bay IPC the same channel can be used for
>    both blocking and non-blocking TX (behavior is controlled by the
>    timeout argument passed to keembay_ipc_send()).
>
> Having said that, I guess that it could be possible to create a Mailbox
> driver implementing the core communication mechanism used by the Keem
> Bay IPC and then build our API around it (basically having two
> drivers). But I'm not sure that would make the code simpler or easier
> to maintain. Any thoughts on this?
>
I think so. Most of KeemBay specific behaviour would be implemented in
the shared channel above the mailbox api.

cheers!