From: Jassi Brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Bjorn Andersson
<bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Jeffrey Hugo <jhugo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
Andy Gross <andy.gross-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
Ohad Ben-Cohen <ohad-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Devicetree List
<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Linux Kernel Mailing List
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
linux-remoteproc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver
Date: Wed, 10 May 2017 08:03:53 +0530 [thread overview]
Message-ID: <CABb+yY2LoX_rOSCkhnp-gS7EHTN+98xNn2E1+z7_7bFjQ2wtoA@mail.gmail.com> (raw)
In-Reply-To: <20170509191106.GQ15143@minitux>
On Wed, May 10, 2017 at 12:41 AM, Bjorn Andersson
<bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue 09 May 09:41 PDT 2017, Jassi Brar wrote:
>>
>> >> >> The client should call mbox_client_txdone() after
>> >> >> mbox_send_message().
>> >> >
>> >> > So every time we call mbox_send_message() from any of the client drivers
>> >> > we also needs to call mbox_client_txdone()?
>> >> >
>> >> Yes.
>> >>
>> >> > This seems like an awkward side effect of using the mailbox framework -
>> >> > which has to be spread out in at least 6 different client drivers :(
>> >> >
>> >> No. Mailbox or whatever you implement - you must (and do) tick the
>> >> state machine to keep the messages moving.
>> >
>> > But the state you have in the other mailbox drivers is not a concern of
>> > the APCS IPC.
>> >
>> No, as you say above you check for space before writing the next
>> message, this is what I call ticking the state machine.
>>
>
> Sure, but you're talking about the mailbox state machine. The APCS IPC
> doesn't have states. The _only_ thing that the APCS IPC provides is a
> mechanism for informing the other side that "hey there's something to
> do". So it doesn't matter if there's already a pending "hey there's
> something to do", because adding another will still only be "hey there's
> something to do".
>
> I'm just trying to describe the big picture, but you keep confusing the
> mailbox/doorbell responsibilities with the client's responsibilities.
>
I think I do understand the bigger picture...
The client driver sets up data packet in SHM and submit a "doorbell"
to be ringed. The controller driver simply sets some bit to trigger an
irq on the remote side (doorbell). And before submitting a "doorbell"
the client makes sure there is some space for data packet to be
written. Right? You see, in the big picture you do have a
state-machine.
[Message to send]
|
|
|-------------->|
| No |
| |
|___[Space Available?]
|
|Yes
|
|
[ Setup Data in SHM]
|
V
[Ring Doorbell]
Mailbox framework supports this whole picture. There is even a
callback (tx_prepare) to setup data packet just before the doorbell is
to be rung.
>> BTW, this is an option only if your client driver doesn't want to
>> explicitly tick the state machine by calling mbox_client_txdone()...
>> which I think should be done in the first place.
>>
>
> There is no state of the APCS IPC, so the overhead is created by the
> mailbox framework.
>
Overhead remains the same if you move the check from your client
drivers to last_tx_done.
OR your client driver, rightfully, drive the state machine by calling
mbox_client_txdone() like other platforms.
[Message to send]<----------|
| |
| |
|-------------->| |
| No | |
| | |
|___[Space Available?] |
| |
|Yes |
| |
V |
[Setup Data in SHM] |
| |
V |
mbox_send_message() |
| |
V |
mbox_client_txdone() |
| |
V______________|
> The part where this piece of hardware differs from the other mailboxes
> is that TX is done as send_data() returns and in the realm of the
> mailbox there is no such thing as "tx done". So how about we extend the
> framework to handle stateless and message-less doorbells?
>
This is a very common usecase. It would be unfair to other platforms
to modify the API just because you find it awkward to call
mbox_client_txdone() right after mbox_send_message(). For example,
drivers/firmware/tegra/bpmp.c
I'd much rather have mbox_send_message_and_tick() than implant a new api.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2017-05-10 2:33 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-04 20:05 [PATCH v4 1/5] mailbox: Make startup and shutdown ops optional Bjorn Andersson
2017-05-04 20:05 ` [PATCH v4 2/5] dt-bindings: mailbox: Introduce Qualcomm APCS global binding Bjorn Andersson
2017-05-04 20:05 ` [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Bjorn Andersson
2017-05-05 10:26 ` Jassi Brar
2017-05-05 18:37 ` Bjorn Andersson
2017-05-05 19:22 ` Jassi Brar
[not found] ` <CABb+yY03imq8911rF9dHJjG_a_aosksA=tgk7Rog5AZSezHDyg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-05 19:53 ` Jeffrey Hugo
2017-05-05 20:22 ` Jassi Brar
2017-05-05 20:39 ` Jeffrey Hugo
[not found] ` <CABb+yY1K3Hyn7CzQCA6tHu-KrPigARc3cYgBdT8PNX7vjhPK0g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-06 1:19 ` Bjorn Andersson
2017-05-06 4:48 ` Jassi Brar
2017-05-08 5:54 ` Bjorn Andersson
2017-05-08 6:47 ` Jassi Brar
2017-05-08 19:11 ` Bjorn Andersson
2017-05-09 16:41 ` Jassi Brar
[not found] ` <CABb+yY2uC6vNFTq1KtZwrrw8x4BEfc2cMjF5DwrLbGpYWBNBzQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-09 19:11 ` Bjorn Andersson
2017-05-10 2:33 ` Jassi Brar [this message]
2017-05-10 19:00 ` Bjorn Andersson
2017-05-11 2:07 ` Jassi Brar
2017-05-12 22:48 ` Bjorn Andersson
2017-05-16 11:25 ` Jassi Brar
2017-05-04 20:05 ` [PATCH v4 4/5] soc: qcom: Add device tree binding for GLINK RPM Bjorn Andersson
2017-05-08 17:06 ` Rob Herring
2017-05-08 17:53 ` Bjorn Andersson
2017-05-04 20:05 ` [PATCH v4 5/5] rpmsg: Introduce Qualcomm RPM glink driver Bjorn Andersson
[not found] ` <20170504200539.27027-1-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-05-05 9:35 ` [PATCH v4 1/5] mailbox: Make startup and shutdown ops optional Sudeep Holla
2017-05-05 10:33 ` Jassi Brar
2017-05-05 18:21 ` Bjorn Andersson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABb+yY2LoX_rOSCkhnp-gS7EHTN+98xNn2E1+z7_7bFjQ2wtoA@mail.gmail.com \
--to=jassisinghbrar-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
--cc=andy.gross-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
--cc=bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=jhugo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
--cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-remoteproc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
--cc=ohad-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).