Re: [PATCH 03/13] opp: Keep track of currently programmed OPP

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.