Hi Maxime,

On Fri, 2020-05-15 at 10:19 +0200, Maxime Ripard wrote:
> Hi Nicolas,
> 
> On Mon, May 04, 2020 at 02:05:47PM +0200, Nicolas Saenz Julienne wrote:
> > Hi Maxime, as always, thanks for the series!
> > Some extra context, and comments below.
> > 
> > On Fri, 2020-04-24 at 17:34 +0200, Maxime Ripard wrote:
> > > The RaspberryPi4 firmware actually exposes more clocks than are currently
> > > handled by the driver and we will need to change some of them directly
> > > based on the pixel rate for the display related clocks, or the load for
> > > the
> > > GPU.
> > > 
> > > This rate change can have a number of side-effects, including adjusting
> > > the
> > > various PLL voltages or the PLL parents. The firmware will also update
> > > those clocks by itself for example if the SoC runs too hot.
> > 
> > To complete this:
> > 
> > RPi4's firmware implements DVFS. Clock frequency and SoC voltage are
> > correlated, if you can determine all clocks are running below a maximum then
> > it
> > should be safe to lower the voltage. Currently, firmware actively monitors
> > arm,
> > core, h264, v3d, isp and hevc to determine what voltage can be reduced to,
> > and
> > if arm increases any of those clocks behind the firmware's back, when it has
> > a
> > lowered voltage, a crash is highly likely.
> > 
> > As pointed out to me by RPi foundation engineers hsm/pixel aren't currently
> > being monitored, as driving a high resolution display also requires a high
> > core
> > clock, which already sets an acceptable min voltage threshold. But that
> > might
> > change if needed.
> > 
> > Ultimately, from the DVFS, the safe way to change clocks from arm would be
> > to
> > always use the firmware interface, which we're far from doing right now. On
> > the
> > other hand, AFAIK not all clocks have a firmware counterpart.
> > 
> > Note that we could also have a word with the RPi foundation and see if
> > disabling DVFS is possible (AFAIK it's not an option right now). Although I
> > personally prefer to support this upstream, aside from the obvious benefits
> > to
> > battery powered use cases, not consuming power unnecessarily is always big
> > plus.
> > 
> > > In order to make Linux play as nice as possible with those constraints, it
> > > makes sense to rely on the firmware clocks as much as possible.
> > 
> > As I comment above, not as simple as it seems. I suggest, for now, we only
> > register the clocks we're going to use and that are likely to be affected by
> > DVFS. hsm being a contender here.
> > 
> > As we'd be settling on a hybrid solution here, which isn't ideal to say the
> > least, I'd like to gather some opinions on whether pushing towards a fully
> > firmware based solution is something you'd like to see.
> 
> Thanks for the summary above, I'll try to adjust that commit log to reflect
> this. As for my opinion, I don't really think that an hybrid approach is
> practical, since that would leave us with weird interactions between the
> firmware (and possibly multiple versinos of it) and the linux driver, and this
> would be pretty hard to maintain in the long run.
> 
> That leaves us either the MMIO-based driver or the firmware-based one, and
> here
> with what you said above, at the moment, the firmware based one is a clear
> winner.

We're on the same page here :)

My remaining concern is the fact there isn't a firmware counterpart to every
clock used right now. But it's something we can work out in the future.

Regards,
Nicolas