All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Osipenko <digetx@gmail.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-pm@vger.kernel.org,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Rafael Wysocki <rjw@rjwysocki.net>,
	Sibi Sankar <sibis@codeaurora.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 03/13] opp: Keep track of currently programmed OPP
Date: Fri, 22 Jan 2021 17:31:41 +0300	[thread overview]
Message-ID: <8af5abe4-fc3f-8ce4-ff14-542754f0275d@gmail.com> (raw)
In-Reply-To: <20210122044532.pc7cpcgy3kjbqmls@vireshk-i7>

22.01.2021 07:45, Viresh Kumar пишет:
> On 22-01-21, 00:41, Dmitry Osipenko wrote:
>> 21.01.2021 14:17, Viresh Kumar пишет:
>>> @@ -1074,15 +1091,18 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
>>>  
>>>  	if (!ret) {
>>>  		ret = _set_opp_bw(opp_table, opp, dev, false);
>>> -		if (!ret)
>>> +		if (!ret) {
>>>  			opp_table->enabled = true;
>>> +			dev_pm_opp_put(old_opp);
>>> +
>>> +			/* Make sure current_opp doesn't get freed */
>>> +			dev_pm_opp_get(opp);
>>> +			opp_table->current_opp = opp;
>>> +		}
>>>  	}
>>
>> I'm a bit surprised that _set_opp_bw() isn't used similarly to
>> _set_opp_voltage() in _generic_set_opp_regulator().
>>
>> I'd expect the BW requirement to be raised before the clock rate goes UP.
> 
> I remember discussing that earlier when this stuff came in, and this I
> believe is the reason for that.
> 
> We need to scale regulators before/after frequency because when we
> increase the frequency a regulator may _not_ be providing enough power
> to sustain that (even for a short while) and this may have undesired
> effects on the hardware and so it is important to prevent that
> malfunction.
> 
> In case of bandwidth such issues will not happen (AFAIK) and doing it
> just once is normally enough. It is just about allowing more data to
> be transmitted, and won't make the hardware behave badly.
> 

This may not be true for all kinds of hardware, a display controller is
one example. If display's pixclock is raised before the memory bandwidth
of the display's memory client, then display controller may get a memory
underflow since it won't be able to fetch memory fast enough and it's
not possible to pause data transmission to display panel, hence display
panel may get out of sync and a full hardware reset will be needed in
order to recover. At least this is the case for NVIDIA Tegra SoCs.

I guess it's not a real problem for any of OPP API users right now, but
this is something to keep in mind.

WARNING: multiple messages have this Message-ID
From: Dmitry Osipenko <digetx@gmail.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Nishanth Menon <nm@ti.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	linux-pm@vger.kernel.org, Stephen Boyd <sboyd@kernel.org>,
	Viresh Kumar <vireshk@kernel.org>,
	Rafael Wysocki <rjw@rjwysocki.net>,
	linux-kernel@vger.kernel.org, Sibi Sankar <sibis@codeaurora.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 03/13] opp: Keep track of currently programmed OPP
Date: Fri, 22 Jan 2021 17:31:41 +0300	[thread overview]
Message-ID: <8af5abe4-fc3f-8ce4-ff14-542754f0275d@gmail.com> (raw)
In-Reply-To: <20210122044532.pc7cpcgy3kjbqmls@vireshk-i7>

22.01.2021 07:45, Viresh Kumar пишет:
> On 22-01-21, 00:41, Dmitry Osipenko wrote:
>> 21.01.2021 14:17, Viresh Kumar пишет:
>>> @@ -1074,15 +1091,18 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
>>>  
>>>  	if (!ret) {
>>>  		ret = _set_opp_bw(opp_table, opp, dev, false);
>>> -		if (!ret)
>>> +		if (!ret) {
>>>  			opp_table->enabled = true;
>>> +			dev_pm_opp_put(old_opp);
>>> +
>>> +			/* Make sure current_opp doesn't get freed */
>>> +			dev_pm_opp_get(opp);
>>> +			opp_table->current_opp = opp;
>>> +		}
>>>  	}
>>
>> I'm a bit surprised that _set_opp_bw() isn't used similarly to
>> _set_opp_voltage() in _generic_set_opp_regulator().
>>
>> I'd expect the BW requirement to be raised before the clock rate goes UP.
> 
> I remember discussing that earlier when this stuff came in, and this I
> believe is the reason for that.
> 
> We need to scale regulators before/after frequency because when we
> increase the frequency a regulator may _not_ be providing enough power
> to sustain that (even for a short while) and this may have undesired
> effects on the hardware and so it is important to prevent that
> malfunction.
> 
> In case of bandwidth such issues will not happen (AFAIK) and doing it
> just once is normally enough. It is just about allowing more data to
> be transmitted, and won't make the hardware behave badly.
> 

This may not be true for all kinds of hardware, a display controller is
one example. If display's pixclock is raised before the memory bandwidth
of the display's memory client, then display controller may get a memory
underflow since it won't be able to fetch memory fast enough and it's
not possible to pause data transmission to display panel, hence display
panel may get out of sync and a full hardware reset will be needed in
order to recover. At least this is the case for NVIDIA Tegra SoCs.

I guess it's not a real problem for any of OPP API users right now, but
this is something to keep in mind.

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

  reply	other threads:[~2021-01-22 14:36 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-21 11:17 [PATCH 00/13] opp: Implement dev_pm_opp_set_opp() Viresh Kumar
2021-01-21 11:17 ` Viresh Kumar
2021-01-21 11:17 ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 01/13] opp: Rename _opp_set_rate_zero() Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 02/13] opp: No need to check clk for errors Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 03/13] opp: Keep track of currently programmed OPP Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 21:41   ` Dmitry Osipenko
2021-01-21 21:41     ` Dmitry Osipenko
2021-01-22  4:45     ` Viresh Kumar
2021-01-22  4:45       ` Viresh Kumar
2021-01-22 14:31       ` Dmitry Osipenko [this message]
2021-01-22 14:31         ` Dmitry Osipenko
2021-01-25  3:12         ` Viresh Kumar
2021-01-25  3:12           ` Viresh Kumar
2021-01-27 16:31       ` Akhil P Oommen
2021-01-28  4:14         ` Viresh Kumar
2021-01-28  4:14           ` Viresh Kumar
2021-07-07 10:24   ` Ionela Voinescu
2021-07-07 10:24     ` Ionela Voinescu
2021-07-08  7:53     ` Viresh Kumar
2021-07-08  7:53       ` Viresh Kumar
2021-07-09  8:57       ` Ionela Voinescu
2021-07-09  8:57         ` Ionela Voinescu
2021-07-12  4:14         ` Viresh Kumar
2021-07-12  4:14           ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 04/13] opp: Split _set_opp() out of dev_pm_opp_set_rate() Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 05/13] opp: Allow _set_opp() to work for non-freq devices Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 06/13] opp: Allow _generic_set_opp_regulator() " Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 07/13] opp: Allow _generic_set_opp_clk_only() " Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 20:26   ` Dmitry Osipenko
2021-01-21 20:26     ` Dmitry Osipenko
2021-01-22  4:35     ` Viresh Kumar
2021-01-22  4:35       ` Viresh Kumar
2021-01-25 21:09       ` Dmitry Osipenko
2021-01-25 21:09         ` Dmitry Osipenko
2021-01-27  6:58         ` Viresh Kumar
2021-01-27  6:58           ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 08/13] opp: Update parameters of _set_opp_custom() Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 09/13] opp: Implement dev_pm_opp_set_opp() Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 10/13] cpufreq: qcom: Migrate to dev_pm_opp_set_opp() Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 11/13] devfreq: tegra30: " Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 21:36   ` Dmitry Osipenko
2021-01-21 21:36     ` Dmitry Osipenko
2021-01-22  6:26     ` Viresh Kumar
2021-01-22  6:26       ` Viresh Kumar
2021-01-22 15:28       ` Dmitry Osipenko
2021-01-22 15:28         ` Dmitry Osipenko
2021-01-25  3:14         ` Viresh Kumar
2021-01-25  3:14           ` Viresh Kumar
2021-01-25 16:00           ` Dmitry Osipenko
2021-01-25 16:00             ` Dmitry Osipenko
2021-01-27  9:10   ` [PATCH V2 " Viresh Kumar
2021-01-27 10:02     ` Viresh Kumar
2021-01-27 15:58       ` Dmitry Osipenko
2021-01-28  7:01         ` Viresh Kumar
2021-02-01  0:21     ` Chanwoo Choi
2021-02-01 19:57     ` Dmitry Osipenko
2021-01-21 11:17 ` [PATCH 12/13] drm: msm: " Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar
2021-01-21 11:17 ` [PATCH 13/13] opp: Remove dev_pm_opp_set_bw() Viresh Kumar
2021-01-21 11:17   ` Viresh Kumar

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=8af5abe4-fc3f-8ce4-ff14-542754f0275d@gmail.com \
    --to=digetx@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=rjw@rjwysocki.net \
    --cc=sboyd@kernel.org \
    --cc=sibis@codeaurora.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    --cc=vireshk@kernel.org \
    --subject='Re: [PATCH 03/13] opp: Keep track of currently programmed OPP' \
    /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

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.