From: Rob Herring <robh@kernel.org>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Jassi Brar <jassisinghbrar@gmail.com>,
Arnd Bergmann <arnd@arndb.de>,
Frank Rowand <frowand.list@gmail.com>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Vincent Guittot <vincent.guittot@linaro.org>,
linux-arm-kernel@lists.infradead.org,
Sudeep Holla <sudeep.holla@arm.com>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC] dt-bindings: mailbox: add doorbell support to ARM MHU
Date: Thu, 28 May 2020 13:20:05 -0600 [thread overview]
Message-ID: <20200528192005.GA494874@bogus> (raw)
In-Reply-To: <0a50f0cf5593baeb628dc8606c523665e5e2ae6c.1589519600.git.viresh.kumar@linaro.org>
On Fri, May 15, 2020 at 10:47:38AM +0530, Viresh Kumar wrote:
> From: Sudeep Holla <sudeep.holla@arm.com>
>
> Hi Rob, Arnd and Jassi,
>
> This stuff has been doing rounds on the mailing list since several years
> now with no agreed conclusion by all the parties. And here is another
> attempt to get some feedback from everyone involved to close this once
> and for ever. Your comments will very much be appreciated.
>
> The ARM MHU is defined here in the TRM [1] for your reference, which
> states following:
>
> "The MHU drives the signal using a 32-bit register, with all 32
> bits logically ORed together. The MHU provides a set of
> registers to enable software to set, clear, and check the status
> of each of the bits of this register independently. The use of
> 32 bits for each interrupt line enables software to provide more
> information about the source of the interrupt. For example, each
> bit of the register can be associated with a type of event that
> can contribute to raising the interrupt."
>
> On few other platforms, like qcom, similar doorbell mechanism is present
> with separate interrupt for each of the bits (that's how I understood
> it), while in case of ARM MHU, there is a single interrupt line for all
> the 32 bits. Also in case of ARM MHU, these registers and interrupts
> have 3 copies for different priority levels, i.e. low priority
> non-secure, high priority non-secure and secure channels.
>
> For ARM MHU, both the dt bindings and the Linux driver support 3
> channels for the different priorities right now and support sending a 32
> bit data on every transfer in a locked fashion, i.e. only one transfer
> can be done at once and the other have to wait for it to finish first.
>
> Here are the point of view of the parties involved on this subject:
>
> Jassi's viewpoint:
>
> - Virtualization of channels should be discouraged in software based on
> specific usecases of the application. This may invite other mailbox
> driver authors to ask for doing virtualization in their drivers.
>
> - In mailbox's terminology, every channel is equivalent to a signal,
> since there is only one signal generated here by the MHU, there should
> be only one channel per priority level.
>
> - The clients should send data (of just setting 1 bit or many in the 32
> bit word) using the existing mechanism as the delays due to
> serialization shouldn't be significant anyway.
>
> - The driver supports 90% of the users with the current implementation
> and it shouldn't be extended to support doorbell and implement two
> different modes by changing value of #mbox-cells field in bindings.
>
> Sudeep (ARM) and myself as well to some extent:
>
> - The hardware gives us the capability to write the register in
> parallel, i.e. we can write 0x800 and 0x400 together without any
> software locks, and so these 32 bits should be considered as separate
> channel even if only one interrupt is issued by the hardware finally.
> This shouldn't be called as virtualization of the channels, as the
> hardware supports this (as clearly mentioned in the TRM) and it takes
> care of handling the signal properly.
>
> - With serialization, if we use only one channel as today at every
> priority, if there are 5 requests to send signal to the receiver and
> the dvfs request is the last one in queue (which may be called from
> scheduler's hot path with fast switching), it unnecessarily needs to
> wait for the first four transfers to finish due to the software
> locking imposed by the mailbox framework. This adds additional delay,
> maybe of few ms only, which isn't required by the hardware but just by
> the software and few ms can be important in scheduler's hotpath.
>
> - With the current approach it isn't possible to assign different bits
> (or doorbell numbers) to clients from DT and the only way of doing
> that without adding new bindings is by extending #mbox-cells to accept
> a value of 2 as done in this patch.
>
> Jassi and Sudeep, I hope I was able to represent both the view points
> properly here. Please correct me if I have made a mistake here.
>
> This is it. It would be nice to get the views of everyone now on this
> and how should this be handled.
I am perfectly fine with adding another cell which seems appropriate
here. You can have 5 cells for all I care if that makes sense for
the h/w. That has nothing to do with the Linux design. Whether Linux
requires serializing mailbox accesses is a separate issue. On that side,
it seems silly to not allow driving the h/w in the most efficient way
possible.
Rob
next prev parent reply other threads:[~2020-05-28 19:20 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-15 5:17 [RFC] dt-bindings: mailbox: add doorbell support to ARM MHU Viresh Kumar
2020-05-15 16:46 ` Jassi Brar
2020-05-18 7:35 ` Viresh Kumar
2020-05-19 0:53 ` Jassi Brar
2020-05-19 4:39 ` Viresh Kumar
2020-05-19 1:29 ` Bjorn Andersson
2020-05-19 3:40 ` Viresh Kumar
2020-05-19 4:05 ` Jassi Brar
2020-06-03 18:31 ` Sudeep Holla
2020-06-03 18:42 ` Jassi Brar
2020-06-03 18:28 ` Sudeep Holla
2020-05-28 19:20 ` Rob Herring [this message]
2020-05-29 4:07 ` Viresh Kumar
2020-06-03 18:04 ` Sudeep Holla
2020-06-03 18:17 ` Sudeep Holla
2020-06-04 5:59 ` Viresh Kumar
2020-06-04 8:28 ` Sudeep Holla
2020-06-03 18:32 ` Jassi Brar
2020-06-04 9:20 ` Sudeep Holla
2020-06-04 15:15 ` Jassi Brar
2020-06-05 4:56 ` Sudeep Holla
2020-06-05 6:30 ` Jassi Brar
2020-06-05 8:58 ` Sudeep Holla
2020-06-05 15:42 ` Jassi Brar
2020-06-10 9:33 ` Viresh Kumar
2020-06-11 10:00 ` Sudeep Holla
2020-06-12 0:34 ` Jassi Brar
2020-06-12 5:28 ` Viresh Kumar
2020-09-08 9:14 ` Arnd Bergmann
2020-09-08 9:27 ` Viresh Kumar
2020-09-08 13:26 ` Sudeep Holla
2020-09-09 3:23 ` Jassi Brar
2020-09-09 4:46 ` Viresh Kumar
2020-09-09 9:31 ` Sudeep Holla
2020-05-29 5:20 ` Jassi Brar
2020-05-29 6:27 ` Viresh Kumar
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=20200528192005.GA494874@bogus \
--to=robh@kernel.org \
--cc=arnd@arndb.de \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=jassisinghbrar@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sudeep.holla@arm.com \
--cc=vincent.guittot@linaro.org \
--cc=viresh.kumar@linaro.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).