Linux-Clk Archive on lore.kernel.org
 help / color / Atom feed
From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
To: Maxime Ripard <maxime@cerno.tech>
Cc: Eric Anholt <eric@anholt.net>,
	Tim Gover <tim.gover@raspberrypi.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-clk@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com,
	linux-rpi-kernel@lists.infradead.org,
	Phil Elwell <phil@raspberrypi.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 20/91] clk: bcm: rpi: Discover the firmware clocks
Date: Thu, 21 May 2020 11:13:35 +0200
Message-ID: <8e5cfb2da29984dc655157b18ef0f34dd576fb21.camel@suse.de> (raw)
In-Reply-To: <20200515081914.lejcqqwezn3zwpft@gilmour.lan>


[-- Attachment #1: Type: text/plain, Size: 3581 bytes --]

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


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply index

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-24 15:33 [PATCH v2 00/91] drm/vc4: Support BCM2711 Display Pipeline Maxime Ripard
2020-04-24 15:33 ` [PATCH v2 03/91] dt-bindings: clock: Add a binding for the RPi Firmware clocks Maxime Ripard
2020-05-11 21:47   ` Rob Herring
2020-05-13  8:13     ` Maxime Ripard
2020-04-24 15:33 ` [PATCH v2 05/91] clk: bcm: rpi: Allow the driver to be probed by DT Maxime Ripard
2020-04-30 16:19   ` Nicolas Saenz Julienne
2020-04-24 15:33 ` [PATCH v2 06/91] clk: bcm: rpi: Statically init clk_init_data Maxime Ripard
2020-05-27  6:47   ` Stephen Boyd
2020-04-24 15:33 ` [PATCH v2 07/91] clk: bcm: rpi: Use clk_hw_register for pllb_arm Maxime Ripard
2020-04-24 15:33 ` [PATCH v2 08/91] clk: bcm: rpi: Remove global pllb_arm clock pointer Maxime Ripard
2020-05-27  6:48   ` Stephen Boyd
2020-04-24 15:33 ` [PATCH v2 09/91] clk: bcm: rpi: Make sure pllb_arm is removed Maxime Ripard
2020-05-27  6:48   ` Stephen Boyd
2020-04-24 15:33 ` [PATCH v2 10/91] clk: bcm: rpi: Remove pllb_arm_lookup global pointer Maxime Ripard
2020-04-24 15:33 ` [PATCH v2 11/91] clk: bcm: rpi: Switch to clk_hw_register_clkdev Maxime Ripard
2020-04-24 15:33 ` [PATCH v2 12/91] clk: bcm: rpi: Make sure the clkdev lookup is removed Maxime Ripard
2020-04-24 15:33 ` [PATCH v2 13/91] clk: bcm: rpi: Create a data structure for the clocks Maxime Ripard
2020-04-24 15:33 ` [PATCH v2 14/91] clk: bcm: rpi: Add clock id to data Maxime Ripard
2020-04-24 15:33 ` [PATCH v2 15/91] clk: bcm: rpi: Pass the clocks data to the firmware function Maxime Ripard
2020-05-27  6:49   ` Stephen Boyd
2020-04-24 15:33 ` [PATCH v2 16/91] clk: bcm: rpi: Rename is_prepared function Maxime Ripard
2020-04-24 15:33 ` [PATCH v2 17/91] clk: bcm: rpi: Split pllb clock hooks Maxime Ripard
2020-04-24 15:33 ` [PATCH v2 18/91] clk: bcm: rpi: Make the PLLB registration function return a clk_hw Maxime Ripard
2020-04-24 15:34 ` [PATCH v2 19/91] clk: bcm: rpi: Add DT provider for the clocks Maxime Ripard
2020-04-24 15:34 ` [PATCH v2 20/91] clk: bcm: rpi: Discover the firmware clocks Maxime Ripard
2020-05-04 12:05   ` Nicolas Saenz Julienne
2020-05-15  8:19     ` Maxime Ripard
2020-05-21  9:13       ` Nicolas Saenz Julienne [this message]
2020-05-27  7:03   ` Stephen Boyd
2020-04-24 15:34 ` [PATCH v2 25/91] clk: bcm: Add BCM2711 DVP driver Maxime Ripard
2020-05-27  7:06   ` Stephen Boyd
2020-04-27  7:23 ` [PATCH v2 00/91] drm/vc4: Support BCM2711 Display Pipelin Jian-Hong Pan
2020-04-28 16:21   ` Maxime Ripard
2020-05-04  6:35     ` Jian-Hong Pan
2020-05-07 17:21       ` Maxime Ripard
2020-05-08  6:20         ` Jian-Hong Pan
2020-05-11  3:12           ` Jian-Hong Pan
2020-05-25 11:11             ` Maxime Ripard
2020-05-26 10:20             ` Maxime Ripard
2020-05-27  3:49               ` Daniel Drake
2020-05-27  9:13                 ` Maxime Ripard
2020-05-27  9:15                   ` Daniel Drake
2020-05-28  7:30                     ` Maxime Ripard

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=8e5cfb2da29984dc655157b18ef0f34dd576fb21.camel@suse.de \
    --to=nsaenzjulienne@suse.de \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=dave.stevenson@raspberrypi.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eric@anholt.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=maxime@cerno.tech \
    --cc=mturquette@baylibre.com \
    --cc=phil@raspberrypi.com \
    --cc=sboyd@kernel.org \
    --cc=tim.gover@raspberrypi.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

Linux-Clk Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-clk/0 linux-clk/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-clk linux-clk/ https://lore.kernel.org/linux-clk \
		linux-clk@vger.kernel.org
	public-inbox-index linux-clk

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-clk


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git