Re: [PATCH v3 01/19] media: venus: pm_helpers: Only set rate of the core clock in core_clks_enable

[Dikshita Agarwal]

On 3/27/2024 11:38 PM, Konrad Dybcio wrote:
> Commit c22b1a29497c ("media: venus: core,pm: Vote for min clk freq
> during venus boot") intended to up the rate of the Venus core clock
> from the XO minimum to something more reasonable, based on the per-
> SoC frequency table.
> 
> Unfortunately, it ended up calling set_rate with that same argument
> on all clocks in res->clks. Fix that using the OPP API.
> 
> Fixes: c22b1a29497c ("media: venus: core,pm: Vote for min clk freq during venus boot")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> ---
>  drivers/media/platform/qcom/venus/pm_helpers.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index 502822059498..8bd0ce4ce69d 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -41,24 +41,23 @@ 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 = core->res->freq_tbl;
> -	unsigned int freq_tbl_size = core->res->freq_tbl_size;
> -	unsigned long freq;
> +	struct dev_pm_opp *opp;
> +	unsigned long freq = 0;
>  	unsigned int i;
>  	int ret;
>  
> -	if (!freq_tbl)
> -		return -EINVAL;
> +	if (core->has_opp_table) {
> +		opp = dev_pm_opp_find_freq_ceil(core->dev, &freq);
> +		if (IS_ERR(opp))
> +			return PTR_ERR(opp);
> +		dev_pm_opp_put(opp);
>  
> -	freq = freq_tbl[freq_tbl_size - 1].freq;
> +		ret = dev_pm_opp_set_rate(core->dev, freq);
> +		if (ret)
> +			return ret;
> +	}
Earlier clk_set_rate is called for only V6 target, this change is calling
it unconditionally. Opp table is available for v4 target as well.
>  
>  	for (i = 0; i < res->clks_num; i++) {
> -		if (IS_V6(core)) {
> -			ret = clk_set_rate(core->clks[i], freq);
> -			if (ret)
> -				goto err;
> -		}
> -
>  		ret = clk_prepare_enable(core->clks[i]);
>  		if (ret)
>  			goto err;
> 

Thanks,
Dikshita