On Thu, Nov 15, 2018 at 01:17:58AM +0000, Krishna Sitaraman wrote:
> Thierry, thanks for looping us in.
>
> Reading this thread, I believe Peter has given a brief overview of
> how we manage memory bus clock along with latency & priority
> information to the memory subsystem for isochronous clients. I
> want to add to this:
> - The total amount of isochronous bandwidth achievable is limited
> and we have to arbitrate the available iso bandwidth at runtime among
> the clients. So our tegra framework provides a mechanism for clients
> to check and then lock a particular iso bandwidth before attempting to
> switch to the desired mode which uses it.
> - The tegra framework also provides mechanism for the isochronous
> clients to set a latency and/or priority information to the memory
> arbitration hardware.
>
> The interconnect framework seems to be a good fit and we might be
> able to make use of it. However there are certain additional
> functionality we want to request or suggest that can help in using
> the interconnects.
>
> Listing them out here:
>
> 1. Isochronous bandwidth manager needs to provide feedback to the
> clients (consumers) to know if a particular iso bandwidth request is
> possible or not before clients can make a definite switch. Example
> Display wanting to know if a mode is possible before switch to the new
> configuration. The interconnect framework needs a method for provider
> to give a feedback to the requesting consumer. A check or is_possible
> request before actual set request.
How would we handle races with other bandwidth requests? Let's say the
display driver requests a check for a certain bandwidth, then gets back
a positive reply, after which it would go make the actual request, but
at that point somebody else might have already requested additional
bandwidth, in turn making the actual request fail.
Would it perhaps be a good idea to just make the actual request fail if
there's not enough bandwidth and allocate the requested bandwidth if it
is available? That way after the consumer gets back a positive reply to
the request it knows that it can use it. On the other hand if the reply
was negative it can downgrade to some other mode and retry.
Any solution that involves two separate steps would probably require
some way of locking the provider until the actual request has been
satisfied, right?
Also, if we do move towards something like this, perhaps a better name
for the function to file these requests would be icc_request() or
something like that. This would make it somewhat more obvious that the
function is actually merely a request, rather than a hard "apply"
operation.
Would it also perhaps be useful to have the function return the maximum
amount of bandwidth available on failure? That could provide useful
hints to the consumers about what modes to downgrade and which will fail
straight away anyway. Then again, this would entail a two step approach,
so the actual request for the lower bandwidth might still fail if some
other consumer requested additional bandwidth since the failure.
> 2. How is the peak_bw actually defined and what is the intended
> usage? Need clarity on this.
> a. A existing implementation from Qualcomm, seems to do a max of
> all peak_bw on their code. Does this mean that all consumers would
> not be using their peak_bw at the same time? Why is it not a sum of
> all peak_bw. So this is not clear to us.
>
> 3. In addition to peak_bw and avg_bw can interconnects support a
> floor request on a clock? We need a floor request for clients which
> are affected by latency and not that much by bandwidth. For example
> cpu is more latency sensitive than bandwidth in some cases. So cpu
> clients set a emc floor based on its current cpu frequency to satisfy
> a minimum latency need.
What would be the difference between a floor request and a request for
peak bandwidth corresponding to that floor frequency? Couldn't the CPU
just register a regular bandwidth request to achieve the same goal? I
mean registering a peak bandwidth that meets the minimum requirements
for the needed latency would ensure that bandwidth never goes below
that value, so it would basically be a floor request. Perhaps this could
also be a special case where peak bandwidth and average bandwidth are
actually the same?
> 4. Request to have tracing as a debug option. On every icc_set()
> call print the path and aggregated avg bw value.
This could presumably be done with ftrace and would be, in my opinion, a
good addition to this framework.
> a. We also want to know what the request from every client is, at a
> given instant, so that we can add testcase to ensure the emc
> calculation code is doing the right thing. Automated tests can make
> use of this.
Would a debugfs interface be useful for this? I could imagine that
something similar to the common clock framework (per-client file with
current requests and an additional bandwidth_summary file with a list
of all requests and a total perhaps) would work very well for this.
> 5. To support latency & priority programming some chips need to
> pass additional parameters apart from bandwidth or latency
> information. Will the interconnects framework support mechanism to
> pass a private struct (downstream defined struct) for the set
> operation? The private struct can be part of the icc_node, and
> programmed by the consumer. This will also help to support any
> future deviations.
That's slightly suboptimal because this is a generic framework, so one
of the goals is that consumer drivers are agnostic of the specific
provider implementation. So adding provider-specific requests doesn't
make much sense because generic consumers couldn't use them anyway. I
suppose this could be made to work for cases where we know that the
provider and consumer are always tightly coupled, but it could make
things fairly complicated.
All of that said, I would consider latency and priority generic enough
concepts to fit into this framework natively. Latency and priority can
be just another type of request that can be registered. Depending on
how this develops it might be better to move away from passing request
parameters individually and instead move to something like a structure
based approach:
struct icc_request {
struct {
unsigned long average;
unsigned long peak;
} bandwidth;
struct {
unsigned long minimum;
unsigned long maximum;
} latency;
unsigned long priority;
};
> 6. When will the latency part of the interconnects framework be
> implemented? What features is it adding?
Since you and Sanjay are most familiar with how this works and what
exactly the requirements are, perhaps this is something that you could
prototype as part of an attempt to implement a Tegra provider for the
interconnect framework? That way you can determine what exactly gets
added based on our requirements and Georgi can provide feedback on the
proposed solution and how it fits into the bigger picture.
Thierry
Attachment:
signature.asc
Description: PGP signature
