On 3/10/21 7:33 PM, Bryan O'Donoghue wrote:
> On 06/03/2021 15:01, Stanimir Varbanov wrote:
>>
>>
>> On 2/23/21 3:25 PM, Stanimir Varbanov wrote:
>>>
>>>
>>> On 2/22/21 6:02 PM, Bryan O'Donoghue wrote:
>>>> From: Dikshita Agarwal <dikshita@xxxxxxxxxxxxxx>
>>>>
>>>> Vote for min clk frequency for core clks during prepare and enable
>>>> clocks
>>>> at boot sequence. Without this the controller clock runs at very low
>>>> value
>>>> (9.6MHz) which is not sufficient to boot venus.
>>>>
>>>> Signed-off-by: Dikshita Agarwal <dikshita@xxxxxxxxxxxxxx>
>>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
>>>> ---
>>>> drivers/media/platform/qcom/venus/pm_helpers.c | 14 ++++++++++++++
>>>> 1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c
>>>> b/drivers/media/platform/qcom/venus/pm_helpers.c
>>>> index 4f5d42662963..767cb00d4b46 100644
>>>> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
>>>> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
>>>> @@ -41,10 +41,24 @@ static int core_clks_get(struct venus_core *core)
>>>> static int core_clks_enable(struct venus_core *core)
>>>> {
>>>> const struct venus_resources *res = core->res;
>>>> + const struct freq_tbl *freq_tbl = NULL;
>>>> + unsigned int freq_tbl_size = 0;
>>>> + unsigned long freq = 0;
>>>
>>> no need to initialize to zero.
>>>
>>>> unsigned int i;
>>>> int ret;
>>>> + freq_tbl = core->res->freq_tbl;
>>>> + freq_tbl_size = core->res->freq_tbl_size;
>>>
>>> could you initialize those at the variables declaration?
>>>
>>>> + if (!freq_tbl)
>>>> + return -EINVAL;
>>>> +
>>>> + freq = freq_tbl[freq_tbl_size - 1].freq;
>>>> +
>>>> for (i = 0; i < res->clks_num; i++) {
>>>> + ret = clk_set_rate(core->clks[i], freq);
>>>
>>> I'm not sure that we have to set the rate for all core->clks[i] ?
>>
>> Confirmed. This produces regressions on db410c (I haven't tested on
>> other platforms yet).
>>
>>>
>>>> + if (ret)
>>>> + goto err;
>>>> +
>>>> ret = clk_prepare_enable(core->clks[i]);
>>>> if (ret)
>>>> goto err;
>>>>
>>>
>>
>
> OK, I have this now on db410c
>
> I made a tree out of
>
> svarbanov-linux-tv/venus-for-next-v5.13
> +
> svarbanov-linux-tv/venus-msm8916-fixes - minor fix here integrating on
> top of 5.13
>
> and then put the sm8250 changes on top of that
>
> https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=tracking-qcomlt-sm8250-venus-integrated-sm8250
>
>
> So confirm db410c works up to tag
> tracking-qcomlt-sm8250-venus-integrated-sm8250-02+svarbanov-linux-tv/venus-msm8916-fixes
>
>
> I'll work on fixing your feedback on that putative branch.
Thanks!
I fixed the regression on db410c by set the rate for the core->clks[0]
only, e.g. the clock at which the remote processor is running.
>
> ---
> bod
--
regards,
Stan
