On Fri, May 15, 2020 at 10:47:38AM +0530, Viresh Kumar wrote: > From: Sudeep Holla <sudeep.holla@xxxxxxx> > > 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
