On Wed, May 05, 2021 at 03:39:17PM +0530, Sibi Sankar wrote:

On 2021-05-05 14:19, Sudeep Holla wrote:

[...]

quoted

But I am not sure how this is related to the above commit anyways.

quoted

I guess your main concern for breakage is ^^ commit? The original
design is
to list a super set of frequencies supported by all variants of the
SoC
along with the required DDR/L3 bandwidth values. When we run into
non-documented frequency we just wouldn't put in bw votes for it which
should be fine since the entire opp_table is optional. If this is
the reason
for the NACK I can try get it reverted with Matthias's ack.

No my main concern is this platform uses "qcom-cpufreq-hw" driver and
the
fact that the OPPs are retrieved from the hardware lookup table
invalidates
whatever we have in DT. In short it will be junk and becomes obsolete.

The table provides mapping to bandwidths which aren't available in the
firmware though. In short we do have to store the mapping somewhere i.e. a
mapping that lists all possible frequencies to its bandwidth requirements
needs to be present and using a opp table with the interconnect bw bindings
was the consensus reached.

I understand and that's exactly what I had pointed out earlier when I
mentioned that I had raised this concern previously.

Given that a duplicate mapping that lists all possible frequencies to bw is
inevitable

I disagree, it was made inevitable by not listening to all feedback, sorry.

and Qualcomm has a way of listing all the supported frequencies for the SoC,
I feel that dt breakage in the future should be a non-concern.

I don't completely understand this TBH. Also my main worry is as we move more
towards abstract scale and/or index based, any addition or deletion of OPPs
results in change in the index or scale. It may be dealt on absolute scale
today everywhere and not a problem *today*, but it will break IMO.

Not sure why you call it junk since it solves the perf/power requirements on
SDM845/SC7180 SoCs. When it becomes obsolete it would mean that they are
better devfreq governors available upstream and that's a good reason for the
opp tables to go away.

Nope, I meant the firmware updates the OPP table underneath for whatever
valid reasons it may have.

quoted

So what I suggested before is still valid. You simply can't have static
OPP tables in the DT for this platform. Do get some boot code to fetch
the same from the h/w LUT and patch to the DT or figure out any other way
to manage dynamically.

moving the logic to boot loader doesn't magically fix your concerns though
(since it would also need a superset of available frequencies).

OK that's interesting. I wanted to fetch the exact list from the hardware
every time.

It will suffer from the same problems with an additional dependency on
firmware propagation in case of breakages which is something you can avoid
for the simple cpu based scaling solution.

IMO the cross dependency is one part of problem and fetching the exact list
of OPPs available for the CPU is another. I meant to fix the latter in boot
code. The former is something I assume DT bindings deals with.

quoted

So NACK still stands for static addition of OPPs to the DT as in this
patch.

I'll let Viresh take the call since this solution is already used on older SoCs.

Sure, definitely I am just expressing my concern with NACK and I don't have
the final say 😁.

--
Regards,
Sudeep