Re: [PATCH v4 4/5] arm64: dts: sdm845: Add OPP tables and power-domains for venus

[Rajendra Nayak]

On 7/28/2020 6:22 AM, Stephen Boyd wrote:
> Quoting Viresh Kumar (2020-07-27 08:38:06)
> > On 27-07-20, 17:38, Rajendra Nayak wrote:
> > > On 7/27/2020 11:23 AM, Rajendra Nayak wrote:
> > > > On 7/24/2020 7:39 PM, Stanimir Varbanov wrote:
> > > > > > > +
> > > > > > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ opp-533000000 {
> > > > > > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ opp-hz = /bits/ 64 <533000000>;
> >
> > Is this the highest OPP in table ?
> >
> > > > > Actually it comes from videocc, where ftbl_video_cc_venus_clk_src
> > > > > defines 533000000 but the real calculated freq is 533000097.
> > > >
> > > > I still don't quite understand why the videocc driver returns this
> > > > frequency despite this not being in the freq table.
> > >
> > > Ok, so I see the same issue on sc7180 also. clk_round_rate() does seem to
> > > return whats in the freq table, but clk_set_rate() goes ahead and sets it
>
> I'm happy to see clk_round_rate() return the actual rate that would be
> achieved and not just the rate that is in the frequency tables. Would
> that fix the problem? 

It would, but only if I also update the OPP table to have 533000097
instead of 533000000 (which I guess is needed anyway)
If this is the actual frequency that's achievable, then perhaps even the clock
freq table should have this? 533000097 and not 533000000?
That way clk_round_rate() would return the actual rate that's achieved and
we don't need any extra math. Isn't that the reason these freq tables exist
anyway.

> It may be that we need to make clk_round_rate() do
> some more math on qcom platforms and actually figure out what the rate
> is going to be instead of blindly trust the frequency that has been set
> in the tables.
>
> > > to 533000097. Subsequently when we try to set a different OPP, it fails to
> > > find the 'current' OPP entry for 533000097. This sounds like an issue with the OPP
> > > framework? Should we not fall back to the highest OPP as the current OPP?
> > >
> > > Stephen/Viresh, any thoughts?
> >
> > I think we (in all frameworks generally) try to set a frequency <=
> > target frequency and so there may be a problem if the frequency is
> > larger than highest supported. IOW, you need to fix tables a bit.
>
> Rounding is annoying for sure.

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation