All of lore.kernel.org
 help / color / mirror / Atom feed
From: Viresh Kumar <viresh.kumar@linaro.org>
To: Rex-BC Chen <rex-bc.chen@mediatek.com>
Cc: rafael@kernel.org, robh+dt@kernel.org, krzk+dt@kernel.org,
	matthias.bgg@gmail.com, jia-wei.chang@mediatek.com,
	roger.lu@mediatek.com, hsinyi@google.com, khilman@baylibre.com,
	angelogioacchino.delregno@collabora.com,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	Project_Global_Chrome_Upstream_Group@mediatek.com
Subject: Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
Date: Thu, 28 Apr 2022 17:18:35 +0530	[thread overview]
Message-ID: <20220428114835.3ktimyz2tzzqdcbg@vireshk-i7> (raw)
In-Reply-To: <346736a339bed576817179ded3795d61f71fa06a.camel@mediatek.com>

On 28-04-22, 19:16, Rex-BC Chen wrote:
> Yes, the call stack will eventually go to __cpufreq_driver_target.
> However, we can observe the mismatch between target_freq and policy-cur 
> with a tiny difference.
> e.g.
> [ 553.065356] cpufreq: target for CPU 0: 500000 kHz, relation 0,
> requested 500000 kHz
> [ 553.066366] cpufreq: target_freq/policy->cur: 500000/499999 kHz

So you are trying to set the frequency to 500 MHz now, but policy->cur says it
is 499 MHz.

> We check the assignment of policy->cur could be either from
> cpufreq_driver->get_intermediate or from cpufreq_driver->get.

policy->cur is set only at two places, in your case:
- CPUFREQ_POSTCHANGE
- cpufreq_online()

From what I understand, it is possible that cpufreq_online() is setting your
frequency to 499999 (once at boot), but as soon as a frequency change has
happened after that, policy->cur should be set to 500 MHz and you should see
this problem only once.

From CPUFREQ_POSTCHANGE notifier, we always set policy->cur from the table
itself, which should be 500000 MHz.

I wonder how you see policy->cur to be 499999 here. Does this happen only once ?
Or repeatedly ?

> But it is strange to have the frequency value like 499999 kHz.
> Is the result of tiny frequency difference expected from your point of
> view?

Clock driver can give this value, that is fine.

> > What do you mean by "voltage pulse" here? What actually happens which
> > you want to avoid.
> > 
> 
> When cpufreq is fixed to lowest opp, "voltage pulse" is a quick voltage
> rising and falling phenomenon which can be observed if 'cpufreq_get' is
> invoked.

Do check if the call is reaching your driver's ->target_index(), it should be
which it should not, ideally.

> Thank you for sharing the correct information.
> Is it possible to get frequency (API) a simple way, like get current
> opp frequency?

Lets dig/debug a bit further and fix this if a real problem exists.

-- 
viresh

WARNING: multiple messages have this Message-ID (diff)
From: Viresh Kumar <viresh.kumar@linaro.org>
To: Rex-BC Chen <rex-bc.chen@mediatek.com>
Cc: rafael@kernel.org, robh+dt@kernel.org, krzk+dt@kernel.org,
	matthias.bgg@gmail.com, jia-wei.chang@mediatek.com,
	roger.lu@mediatek.com, hsinyi@google.com, khilman@baylibre.com,
	angelogioacchino.delregno@collabora.com,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	Project_Global_Chrome_Upstream_Group@mediatek.com
Subject: Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
Date: Thu, 28 Apr 2022 17:18:35 +0530	[thread overview]
Message-ID: <20220428114835.3ktimyz2tzzqdcbg@vireshk-i7> (raw)
In-Reply-To: <346736a339bed576817179ded3795d61f71fa06a.camel@mediatek.com>

On 28-04-22, 19:16, Rex-BC Chen wrote:
> Yes, the call stack will eventually go to __cpufreq_driver_target.
> However, we can observe the mismatch between target_freq and policy-cur 
> with a tiny difference.
> e.g.
> [ 553.065356] cpufreq: target for CPU 0: 500000 kHz, relation 0,
> requested 500000 kHz
> [ 553.066366] cpufreq: target_freq/policy->cur: 500000/499999 kHz

So you are trying to set the frequency to 500 MHz now, but policy->cur says it
is 499 MHz.

> We check the assignment of policy->cur could be either from
> cpufreq_driver->get_intermediate or from cpufreq_driver->get.

policy->cur is set only at two places, in your case:
- CPUFREQ_POSTCHANGE
- cpufreq_online()

From what I understand, it is possible that cpufreq_online() is setting your
frequency to 499999 (once at boot), but as soon as a frequency change has
happened after that, policy->cur should be set to 500 MHz and you should see
this problem only once.

From CPUFREQ_POSTCHANGE notifier, we always set policy->cur from the table
itself, which should be 500000 MHz.

I wonder how you see policy->cur to be 499999 here. Does this happen only once ?
Or repeatedly ?

> But it is strange to have the frequency value like 499999 kHz.
> Is the result of tiny frequency difference expected from your point of
> view?

Clock driver can give this value, that is fine.

> > What do you mean by "voltage pulse" here? What actually happens which
> > you want to avoid.
> > 
> 
> When cpufreq is fixed to lowest opp, "voltage pulse" is a quick voltage
> rising and falling phenomenon which can be observed if 'cpufreq_get' is
> invoked.

Do check if the call is reaching your driver's ->target_index(), it should be
which it should not, ideally.

> Thank you for sharing the correct information.
> Is it possible to get frequency (API) a simple way, like get current
> opp frequency?

Lets dig/debug a bit further and fix this if a real problem exists.

-- 
viresh

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
From: Viresh Kumar <viresh.kumar@linaro.org>
To: Rex-BC Chen <rex-bc.chen@mediatek.com>
Cc: rafael@kernel.org, robh+dt@kernel.org, krzk+dt@kernel.org,
	matthias.bgg@gmail.com, jia-wei.chang@mediatek.com,
	roger.lu@mediatek.com, hsinyi@google.com, khilman@baylibre.com,
	angelogioacchino.delregno@collabora.com,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	Project_Global_Chrome_Upstream_Group@mediatek.com
Subject: Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
Date: Thu, 28 Apr 2022 17:18:35 +0530	[thread overview]
Message-ID: <20220428114835.3ktimyz2tzzqdcbg@vireshk-i7> (raw)
In-Reply-To: <346736a339bed576817179ded3795d61f71fa06a.camel@mediatek.com>

On 28-04-22, 19:16, Rex-BC Chen wrote:
> Yes, the call stack will eventually go to __cpufreq_driver_target.
> However, we can observe the mismatch between target_freq and policy-cur 
> with a tiny difference.
> e.g.
> [ 553.065356] cpufreq: target for CPU 0: 500000 kHz, relation 0,
> requested 500000 kHz
> [ 553.066366] cpufreq: target_freq/policy->cur: 500000/499999 kHz

So you are trying to set the frequency to 500 MHz now, but policy->cur says it
is 499 MHz.

> We check the assignment of policy->cur could be either from
> cpufreq_driver->get_intermediate or from cpufreq_driver->get.

policy->cur is set only at two places, in your case:
- CPUFREQ_POSTCHANGE
- cpufreq_online()

From what I understand, it is possible that cpufreq_online() is setting your
frequency to 499999 (once at boot), but as soon as a frequency change has
happened after that, policy->cur should be set to 500 MHz and you should see
this problem only once.

From CPUFREQ_POSTCHANGE notifier, we always set policy->cur from the table
itself, which should be 500000 MHz.

I wonder how you see policy->cur to be 499999 here. Does this happen only once ?
Or repeatedly ?

> But it is strange to have the frequency value like 499999 kHz.
> Is the result of tiny frequency difference expected from your point of
> view?

Clock driver can give this value, that is fine.

> > What do you mean by "voltage pulse" here? What actually happens which
> > you want to avoid.
> > 
> 
> When cpufreq is fixed to lowest opp, "voltage pulse" is a quick voltage
> rising and falling phenomenon which can be observed if 'cpufreq_get' is
> invoked.

Do check if the call is reaching your driver's ->target_index(), it should be
which it should not, ideally.

> Thank you for sharing the correct information.
> Is it possible to get frequency (API) a simple way, like get current
> opp frequency?

Lets dig/debug a bit further and fix this if a real problem exists.

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-04-28 11:48 UTC|newest]

Thread overview: 141+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
2022-04-22  7:52 ` Rex-BC Chen
2022-04-22  7:52 ` Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  8:21   ` AngeloGioacchino Del Regno
2022-04-22  8:21     ` AngeloGioacchino Del Regno
2022-04-22  8:21     ` AngeloGioacchino Del Regno
2022-04-22 17:26   ` Krzysztof Kozlowski
2022-04-22 17:26     ` Krzysztof Kozlowski
2022-04-22 17:26     ` Krzysztof Kozlowski
2022-04-22 17:34     ` Krzysztof Kozlowski
2022-04-22 17:34       ` Krzysztof Kozlowski
2022-04-22 17:34       ` Krzysztof Kozlowski
2022-04-25  6:19       ` Rex-BC Chen
2022-04-25  6:19         ` Rex-BC Chen
2022-04-25  6:19         ` Rex-BC Chen
2022-04-25  8:55         ` Krzysztof Kozlowski
2022-04-25  8:55           ` Krzysztof Kozlowski
2022-04-25  8:55           ` Krzysztof Kozlowski
2022-04-25 10:20           ` Rex-BC Chen
2022-04-25 10:20             ` Rex-BC Chen
2022-04-25 10:20             ` Rex-BC Chen
2022-04-25 10:52             ` Krzysztof Kozlowski
2022-04-25 10:52               ` Krzysztof Kozlowski
2022-04-25 10:52               ` Krzysztof Kozlowski
2022-04-26  8:26               ` Rex-BC Chen
2022-04-26  8:26                 ` Rex-BC Chen
2022-04-26  8:26                 ` Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 02/14] cpufreq: mediatek: Use device print to show logs Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-25  5:10   ` Viresh Kumar
2022-04-25  5:10     ` Viresh Kumar
2022-04-25  5:10     ` Viresh Kumar
2022-04-22  7:52 ` [PATCH V4 03/14] cpufreq: mediatek: Replace old_* with pre_* Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-25  5:11   ` Viresh Kumar
2022-04-25  5:11     ` Viresh Kumar
2022-04-25  5:11     ` Viresh Kumar
2022-04-22  7:52 ` [PATCH V4 04/14] cpufreq: mediatek: Record previous target vproc value Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  8:21   ` AngeloGioacchino Del Regno
2022-04-22  8:21     ` AngeloGioacchino Del Regno
2022-04-22  8:21     ` AngeloGioacchino Del Regno
2022-04-25  5:12     ` Viresh Kumar
2022-04-25  5:12       ` Viresh Kumar
2022-04-25  5:12       ` Viresh Kumar
2022-04-22  7:52 ` [PATCH V4 05/14] cpufreq: mediatek: Add opp notification support Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-25  5:20   ` Viresh Kumar
2022-04-25  5:20     ` Viresh Kumar
2022-04-25  5:20     ` Viresh Kumar
2022-04-25  7:28     ` Rex-BC Chen
2022-04-25  7:28       ` Rex-BC Chen
2022-04-25 10:38     ` Rex-BC Chen
2022-04-25 10:38       ` Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 06/14] cpufreq: mediatek: Move voltage limits to platform data Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 07/14] cpufreq: mediatek: Add .get function Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  8:21   ` AngeloGioacchino Del Regno
2022-04-22  8:21     ` AngeloGioacchino Del Regno
2022-04-22  8:21     ` AngeloGioacchino Del Regno
2022-04-25  5:35   ` Viresh Kumar
2022-04-25  5:35     ` Viresh Kumar
2022-04-25  5:35     ` Viresh Kumar
2022-04-25  9:34     ` Rex-BC Chen
2022-04-25  9:34       ` Rex-BC Chen
2022-04-25  9:34       ` Rex-BC Chen
2022-04-25 10:00       ` Viresh Kumar
2022-04-25 10:00         ` Viresh Kumar
2022-04-25 10:00         ` Viresh Kumar
2022-04-26 11:13         ` Rex-BC Chen
2022-04-26 11:13           ` Rex-BC Chen
2022-04-26 11:13           ` Rex-BC Chen
2022-04-27  3:11           ` Viresh Kumar
2022-04-27  3:11             ` Viresh Kumar
2022-04-27  3:11             ` Viresh Kumar
2022-04-28 11:16             ` Rex-BC Chen
2022-04-28 11:16               ` Rex-BC Chen
2022-04-28 11:16               ` Rex-BC Chen
2022-04-28 11:48               ` Viresh Kumar [this message]
2022-04-28 11:48                 ` Viresh Kumar
2022-04-28 11:48                 ` Viresh Kumar
2022-05-03 11:33                 ` Rex-BC Chen
2022-05-03 11:33                   ` Rex-BC Chen
2022-05-03 11:33                   ` Rex-BC Chen
2022-05-04  8:22                   ` Viresh Kumar
2022-05-04  8:22                     ` Viresh Kumar
2022-05-04  8:22                     ` Viresh Kumar
2022-05-04 11:57                     ` Rex-BC Chen
2022-05-04 11:57                       ` Rex-BC Chen
2022-05-04 11:57                       ` Rex-BC Chen
2022-05-04 11:58                       ` Viresh Kumar
2022-05-04 11:58                         ` Viresh Kumar
2022-05-04 11:58                         ` Viresh Kumar
2022-05-04 12:03                         ` Rex-BC Chen
2022-05-04 12:03                           ` Rex-BC Chen
2022-05-04 12:03                           ` Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 08/14] cpufreq: mediatek: Make sram regulator optional Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-25  5:36   ` Viresh Kumar
2022-04-25  5:36     ` Viresh Kumar
2022-04-25  5:36     ` Viresh Kumar
2022-04-22  7:52 ` [PATCH V4 09/14] cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking() Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  8:20   ` AngeloGioacchino Del Regno
2022-04-22  8:20     ` AngeloGioacchino Del Regno
2022-04-22  8:20     ` AngeloGioacchino Del Regno
2022-04-22  7:52 ` [PATCH V4 10/14] cpufreq: mediatek: Link CCI device to CPU Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 11/14] cpufreq: mediatek: Add support for MT8186 Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 12/14] arm64: dts: mediatek: Add opp table and clock property for MT8183 cpufreq Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  8:20   ` AngeloGioacchino Del Regno
2022-04-22  8:20     ` AngeloGioacchino Del Regno
2022-04-22  8:20     ` AngeloGioacchino Del Regno
2022-04-22  7:52 ` [PATCH V4 13/14] arm64: dts: mediatek: Add MediaTek CCI node for MT8183 Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 14/14] arm64: dts: mediatek: Add mediatek,cci property for MT8183 cpufreq Rex-BC Chen
2022-04-22  7:52   ` [PATCH V4 14/14] arm64: dts: mediatek: Add mediatek, cci " Rex-BC Chen
2022-04-22  7:52   ` Rex-BC Chen
2022-04-22 17:23 ` [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Krzysztof Kozlowski
2022-04-22 17:23   ` Krzysztof Kozlowski
2022-04-22 17:23   ` Krzysztof Kozlowski
2022-04-25  6:20   ` Rex-BC Chen
2022-04-25  6:20     ` Rex-BC Chen
2022-04-25  6:20     ` Rex-BC Chen

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=20220428114835.3ktimyz2tzzqdcbg@vireshk-i7 \
    --to=viresh.kumar@linaro.org \
    --cc=Project_Global_Chrome_Upstream_Group@mediatek.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=devicetree@vger.kernel.org \
    --cc=hsinyi@google.com \
    --cc=jia-wei.chang@mediatek.com \
    --cc=khilman@baylibre.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=rafael@kernel.org \
    --cc=rex-bc.chen@mediatek.com \
    --cc=robh+dt@kernel.org \
    --cc=roger.lu@mediatek.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.