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

Rajendra Nayak
2020-07-28T04:17:37Z

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