Linux-Media Archive mirror
 help / color / mirror / Atom feed
From: Vikash Garodia <quic_vgarodia@quicinc.com>
To: Dikshita Agarwal <quic_dikshita@quicinc.com>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Stanimir Varbanov <stanimir.k.varbanov@gmail.com>,
	Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
	Andy Gross <agross@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	"Philipp Zabel" <p.zabel@pengutronix.de>
Cc: Marijn Suijten <marijn.suijten@somainline.org>,
	Stanimir Varbanov <stanimir.varbanov@linaro.org>,
	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	<linux-media@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 01/19] media: venus: pm_helpers: Only set rate of the core clock in core_clks_enable
Date: Fri, 5 Apr 2024 14:19:57 +0530	[thread overview]
Message-ID: <b960add1-df5f-ced3-74a8-ba4272cae0f3@quicinc.com> (raw)
In-Reply-To: <988b641e-120a-9cb7-f35a-512f865aa6bd@quicinc.com>

Hi Konrad,

On 4/5/2024 1:01 PM, Dikshita Agarwal wrote:
> 
> 
> 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.
It called with same argument, but not with same frequency. The argument is
platform specific and would have different values. Do not see it fixing anything
in existing code, so "Fixes" does not look applicable here. OR, am i  missing
something ?

Thanks,
Vikash
>>
>> Fixes: c22b1a29497c ("media: venus: core,pm: Vote for min clk freq during venus boot")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>  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

  reply	other threads:[~2024-04-05  8:50 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-27 18:08 [PATCH v3 00/19] Venus cleanups Konrad Dybcio
2024-03-27 18:08 ` [PATCH v3 01/19] media: venus: pm_helpers: Only set rate of the core clock in core_clks_enable Konrad Dybcio
2024-04-05  7:31   ` Dikshita Agarwal
2024-04-05  8:49     ` Vikash Garodia [this message]
2024-03-27 18:08 ` [PATCH v3 02/19] media: venus: pm_helpers: Rename core_clks_get to venus_clks_get Konrad Dybcio
2024-04-05  7:32   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 03/19] media: venus: pm_helpers: Add kerneldoc to venus_clks_get() Konrad Dybcio
2024-04-05  8:26   ` Dikshita Agarwal
2024-04-05 12:44     ` Vikash Garodia
2024-04-09 18:22       ` Konrad Dybcio
2024-04-10 12:03         ` Vikash Garodia
2024-04-09 18:16     ` Konrad Dybcio
2024-03-27 18:08 ` [PATCH v3 04/19] media: venus: core: Set OPP clkname in a common code path Konrad Dybcio
2024-04-05  7:39   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 05/19] media: venus: pm_helpers: Kill dead code Konrad Dybcio
2024-04-05  7:49   ` Dikshita Agarwal
2024-04-09 18:24     ` Konrad Dybcio
2024-04-23  7:59       ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 06/19] media: venus: pm_helpers: Move reset acquisition to common code Konrad Dybcio
2024-04-05  7:51   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 07/19] media: venus: core: Deduplicate OPP genpd names Konrad Dybcio
2024-04-05  7:52   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 08/19] media: venus: core: Get rid of vcodec_num Konrad Dybcio
2024-04-05  9:18   ` Dikshita Agarwal
2024-04-05 12:30     ` Vikash Garodia
2024-03-27 18:08 ` [PATCH v3 09/19] media: venus: core: Drop cache properties in resource struct Konrad Dybcio
2024-04-05  7:57   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 10/19] media: venus: core: Use GENMASK for dma_mask Konrad Dybcio
2024-04-05  7:59   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 11/19] media: venus: core: Remove cp_start Konrad Dybcio
2024-04-05  8:09   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 12/19] media: venus: pm_helpers: Commonize core_power Konrad Dybcio
2024-04-05  8:12   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 13/19] media: venus: pm_helpers: Remove pm_ops->core_put Konrad Dybcio
2024-04-05  9:01   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 14/19] media: venus: core: Define a pointer to core->res Konrad Dybcio
2024-04-24  0:33   ` Bryan O'Donoghue
2024-04-25 12:38   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 15/19] media: venus: pm_helpers: Simplify vcodec clock handling Konrad Dybcio
2024-04-25 12:44   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 16/19] media: venus: pm_helpers: Commonize getting clocks and GenPDs Konrad Dybcio
2024-04-25 12:46   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 17/19] media: venus: pm_helpers: Commonize vdec_get() Konrad Dybcio
2024-04-25 12:47   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 18/19] media: venus: pm_helpers: Commonize venc_get() Konrad Dybcio
2024-04-25 12:47   ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 19/19] media: venus: pm_helpers: Use reset_bulk API Konrad Dybcio
2024-04-05  8:30   ` Dikshita Agarwal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b960add1-df5f-ced3-74a8-ba4272cae0f3@quicinc.com \
    --to=quic_vgarodia@quicinc.com \
    --cc=agross@kernel.org \
    --cc=andersson@kernel.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=konrad.dybcio@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=mchehab+huawei@kernel.org \
    --cc=mchehab@kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=quic_dikshita@quicinc.com \
    --cc=stanimir.k.varbanov@gmail.com \
    --cc=stanimir.varbanov@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).