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;
>>
>
--
regards,
Stan
