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

Stephen Boyd
2020-07-28T19:54:37Z

Quoting Rajendra Nayak (2020-07-27 21:17:28)
>
> 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.

Yes the freq tables are there in the clk driver so we don't have to do a
bunch of math. Fixing them to be accurate has been deemed "hard" from
what I recall because the tables are generated from some math function
that truncates the lower Hertz values.