From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
To: Maxime Ripard <maxime@cerno.tech>
Cc: linux-rpi-kernel@lists.infradead.org,
bcm-kernel-feedback-list@broadcom.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org,
Dave Stevenson <dave.stevenson@raspberrypi.com>,
Tim Gover <tim.gover@raspberrypi.com>,
Phil Elwell <phil@raspberrypi.com>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>,
linux-clk@vger.kernel.org
Subject: Re: [PATCH v3 21/25] clk: bcm: rpi: Discover the firmware clocks
Date: Fri, 05 Jun 2020 14:45:50 +0200 [thread overview]
Message-ID: <a8741e98ce8ef4522def822e830de7d8b4a4604a.camel@suse.de> (raw)
In-Reply-To: <d3791b14fceec639085ff427e934852f275e348c.1590594293.git-series.maxime@cerno.tech>
[-- Attachment #1: Type: text/plain, Size: 12850 bytes --]
Hi Maxime,
On Wed, 2020-05-27 at 17:45 +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.
>
> Since the firmware implements DVFS, this rate change can have a number of
> side-effects, including adjusting the various PLL voltages or the PLL
> parents. The firmware also implements thermal throttling, so even some
> thermal pressure can change those parameters behind Linux back.
>
> DVFS is currently implemented on the arm, core, h264, v3d, isp and hevc
> clocks, so updating any of them using the MMIO driver (and thus behind the
> firmware's back) can lead to troubles, the arm clock obviously being the
> most problematic.
>
> 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. However,
> the firmware doesn't seem to provide some equivalents to their MMIO
> counterparts, so we can't really replace that driver entirely.
>
> Fortunately, the firmware has an interface to discover the clocks it
> exposes.
>
> Let's use it to discover, register the clocks in the clocks framework and
> then expose them through the device tree for consumers to use them.
>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
> drivers/clk/bcm/clk-raspberrypi.c | 151 ++++++++++++++++++++--
> include/soc/bcm2835/raspberrypi-firmware.h | 5 +-
> 2 files changed, 144 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-
> raspberrypi.c
> index eebd16040f8a..8d3acf3ee053 100644
> --- a/drivers/clk/bcm/clk-raspberrypi.c
> +++ b/drivers/clk/bcm/clk-raspberrypi.c
> @@ -296,6 +296,142 @@ static struct clk_hw
> *raspberrypi_register_pllb_arm(struct raspberrypi_clk *rpi)
> return &raspberrypi_clk_pllb_arm.hw;
> }
>
> +static int raspberrypi_fw_dumb_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + /*
> + * The firmware will do the rounding but that isn't part of
> + * the interface with the firmware, so we just do our best
> + * here.
> + */
> + req->rate = clamp(req->rate, req->min_rate, req->max_rate);
> + return 0;
> +}
> +
> +static const struct clk_ops raspberrypi_firmware_clk_ops = {
> + .is_prepared = raspberrypi_fw_is_prepared,
> + .recalc_rate = raspberrypi_fw_get_rate,
> + .determine_rate = raspberrypi_fw_dumb_determine_rate,
> + .set_rate = raspberrypi_fw_set_rate,
> +};
> +
> +static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
> + unsigned int parent,
> + unsigned int id)
> +{
> + struct raspberrypi_clk_data *data;
> + struct clk_init_data init = {};
> + u32 min_rate, max_rate;
> + int ret;
> +
> + if (id == RPI_FIRMWARE_ARM_CLK_ID) {
> + struct clk_hw *hw;
> +
> + hw = raspberrypi_register_pllb(rpi);
> + if (IS_ERR(hw)) {
> + dev_err(rpi->dev, "Failed to initialize pllb, %ld\n",
> + PTR_ERR(hw));
> + return hw;
> + }
> +
> + return raspberrypi_register_pllb_arm(rpi);
> + }
> +
> + data = devm_kzalloc(rpi->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return ERR_PTR(-ENOMEM);
> + data->rpi = rpi;
> + data->id = id;
> +
> + init.name = devm_kasprintf(rpi->dev, GFP_KERNEL, "fw-clk-%u", id);
> + init.ops = &raspberrypi_firmware_clk_ops;
> + init.flags = CLK_GET_RATE_NOCACHE;
> +
> + data->hw.init = &init;
> +
> + ret = raspberrypi_clock_property(rpi->firmware, data,
> + RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
> + &min_rate);
> + if (ret) {
> + dev_err(rpi->dev, "Failed to get clock %d min freq: %d",
> + id, ret);
> + return ERR_PTR(ret);
> + }
> +
> + ret = raspberrypi_clock_property(rpi->firmware, data,
> + RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
> + &max_rate);
> + if (ret) {
> + dev_err(rpi->dev, "Failed to get clock %d max freq: %d\n",
> + id, ret);
> + return ERR_PTR(ret);
> + }
> +
> + dev_info(rpi->dev, "Clock %d frequency range: min %u, max %u\n",
> + id, min_rate, max_rate);
This outputs:
[ 9.071965] raspberrypi-clk soc:firmware:clocks: Clock arm frequency range: min 600000000, max 1500000000
[ 9.086115] raspberrypi-clk soc:firmware:clocks: Clock core frequency range: min 200000000, max 500000000
[ 9.102418] raspberrypi-clk soc:firmware:clocks: Clock v3d frequency range: min 250000000, max 500000000
[ 9.120260] raspberrypi-clk soc:firmware:clocks: Clock m2mc frequency range: min 0, max 600000000
I think, arm aside, it's pretty useless. I'd either print it only for arm or
drop it altogether.
> +
> + ret = devm_clk_hw_register(rpi->dev, &data->hw);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + clk_hw_set_rate_range(&data->hw, min_rate, max_rate);
> +
> + if (id == RPI_FIRMWARE_ARM_CLK_ID) {
> + ret = devm_clk_hw_register_clkdev(rpi->dev, &data->hw,
> + NULL, "cpu0");
> + if (ret) {
> + dev_err(rpi->dev, "Failed to initialize clkdev\n");
> + return ERR_PTR(ret);
> + }
> + }
> +
> + return &data->hw;
> +}
> +
> +static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi,
> + struct clk_hw_onecell_data *data)
> +{
> + struct rpi_firmware_get_clocks_response *clks;
> + int ret;
> +
> + clks = devm_kcalloc(rpi->dev,
> + sizeof(*clks), RPI_FIRMWARE_NUM_CLK_ID,
> + GFP_KERNEL);
> + if (!clks)
> + return -ENOMEM;
> +
> + ret = rpi_firmware_property(rpi->firmware, RPI_FIRMWARE_GET_CLOCKS,
> + clks,
> + sizeof(*clks) * RPI_FIRMWARE_NUM_CLK_ID);
> + if (ret)
> + return ret;
> +
> + while (clks->id) {
> + struct clk_hw *hw;
> +
> + switch (clks->id) {
> + case RPI_FIRMWARE_ARM_CLK_ID:
> + case RPI_FIRMWARE_CORE_CLK_ID:
> + case RPI_FIRMWARE_M2MC_CLK_ID:
> + case RPI_FIRMWARE_V3D_CLK_ID:
> + hw = raspberrypi_clk_register(rpi, clks->parent,
> + clks->id);
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> +
> + data->hws[clks->id] = hw;
> + data->num = clks->id + 1;
> + fallthrough;
> +
> + default:
> + clks++;
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int raspberrypi_clk_probe(struct platform_device *pdev)
> {
> struct clk_hw_onecell_data *clk_data;
> @@ -303,7 +439,6 @@ static int raspberrypi_clk_probe(struct platform_device
> *pdev)
> struct device *dev = &pdev->dev;
> struct rpi_firmware *firmware;
> struct raspberrypi_clk *rpi;
> - struct clk_hw *hw;
> int ret;
>
> /*
> @@ -340,17 +475,9 @@ static int raspberrypi_clk_probe(struct platform_device
> *pdev)
> if (!clk_data)
> return -ENOMEM;
>
> - hw = raspberrypi_register_pllb(rpi);
> - if (IS_ERR(hw)) {
> - dev_err(dev, "Failed to initialize pllb, %ld\n", PTR_ERR(hw));
> - return PTR_ERR(hw);
> - }
> -
> - hw = raspberrypi_register_pllb_arm(rpi);
> - if (IS_ERR(hw))
> - return PTR_ERR(hw);
> - clk_data->hws[RPI_FIRMWARE_ARM_CLK_ID] = hw;
> - clk_data->num = RPI_FIRMWARE_ARM_CLK_ID + 1;
> + ret = raspberrypi_discover_clocks(rpi, clk_data);
> + if (ret)
> + return ret;
>
> ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> clk_data);
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h
> b/include/soc/bcm2835/raspberrypi-firmware.h
> index 3025aca3c358..1c48f8269eab 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -136,6 +136,11 @@ enum rpi_firmware_property_tag {
> RPI_FIRMWARE_GET_DMA_CHANNELS = 0x00060001,
> };
>
> +struct rpi_firmware_get_clocks_response {
> + __le32 parent;
> + __le32 id;
> +};
> +
As per other firmware consumer drivers, this should be moved into
clk-raspberrypi.c. Also I'd switch to using u32. It's pretty clear this will be
used only on this platform, so no need to worry about endianness (also common
practice among rpi firmware consumer drivers).
> #if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
> int rpi_firmware_property(struct rpi_firmware *fw,
> u32 tag, void *data, size_t len);
Here's the clk_summary output:
enable prepare protect duty
clock count count count rate accuracy phase cycle
---------------------------------------------------------------------------------------------
fw-clk-m2mc 0 0 0 149984992 0 0 50000
fw-clk-v3d 0 0 0 250000000 0 0 50000
fw-clk-core 0 0 0 200000000 0 0 50000
fw-clk-arm 0 0 0 1500000000 0 0 50000
otg 0 0 0 480000000 0 0 50000
osc 2 2 2 54000000 0 0 50000
tsens 0 0 0 3375000 0 0 50000
otp 0 0 0 13500000 0 0 50000
timer 0 0 0 1000000 0 0 50000
plld 1 1 1 2999999988 0 0 50000
plld_dsi1 0 0 0 11718750 0 0 50000
plld_dsi0 0 0 0 11718750 0 0 50000
plld_per 5 5 4 749999997 0 0 50000
pwm 1 1 1 9999999 0 0 50000
emmc2 1 1 1 99999999 0 0 50000
emmc 1 1 1 249999999 0 0 50000
uart 1 1 1 47999999 0 0 50000
plld_core 0 0 0 599999998 0 0 50000
pllc 1 1 1 3000000091 0 0 50000
pllc_per 0 0 0 600000019 0 0 50000
pllc_core2 0 0 0 11718751 0 0 50000
pllc_core1 0 0 0 11718751 0 0 50000
pllc_core0 1 1 1 1000000031 0 0 50000
vpu 5 5 2 200000006 0 0 50000
fe804000.i2c_div 1 1 1 99901 0 0 50000
fe205000.i2c_div 1 1 1 99901 0 0 50000
aux_spi2 0 0 0 200000006 0 0 50000
aux_spi1 0 0 0 200000006 0 0 50000
aux_uart 1 1 0 200000006 0 0 50000
peri_image 0 0 0 200000006 0 0 50000
pllb 0 0 0 2999999988 0 0 50000
pllb_arm 0 0 0 599999998 0 0 50000
plla 0 0 0 2999999988 0 0 50000
plla_ccp2 0 0 0 11718750 0 0 50000
plla_dsi0 0 0 0 11718750 0 0 50000
plla_per 0 0 0 11718750 0 0 50000
plla_core 0 0 0 499999998 0 0 50000
h264 0 0 0 249999999 0 0 50000
isp 0 0 0 249999999 0 0 50000
v3d 0 0 0 249999999 0 0 50000
[...]
arm clocks don't match. CCF is caching the mmio version of the clocks, I think
we need to add CLK_GET_RATE_NOCACHE to every clock that's being handled by
firmware.
Otherwise I verified that cpufreq registers/behaves as expected.
Regards,
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2020-06-05 12:45 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-27 15:44 [PATCH v3 00/25] clk: bcm: rpi: Add support for BCM2711 firmware clocks Maxime Ripard
2020-05-27 15:44 ` [PATCH v3 01/25] dt-bindings: arm: bcm: Convert BCM2835 firmware binding to YAML Maxime Ripard
2020-05-27 15:44 ` [PATCH v3 02/25] dt-bindings: clock: Add a binding for the RPi Firmware clocks Maxime Ripard
2020-05-29 18:14 ` Rob Herring
2020-05-29 21:17 ` Stephen Boyd
2020-05-30 16:23 ` Maxime Ripard
2020-05-27 15:44 ` [PATCH v3 03/25] firmware: rpi: Only create clocks device if we don't have a node for it Maxime Ripard
2020-06-04 17:50 ` Nicolas Saenz Julienne
2020-05-27 15:45 ` [PATCH v3 04/25] clk: bcm: rpi: Allow the driver to be probed by DT Maxime Ripard
2020-05-29 21:17 ` Stephen Boyd
2020-06-04 17:52 ` Nicolas Saenz Julienne
2020-05-27 15:45 ` [PATCH v3 05/25] clk: bcm: rpi: Statically init clk_init_data Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 06/25] clk: bcm: rpi: Use clk_hw_register for pllb_arm Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 07/25] clk: bcm: rpi: Remove global pllb_arm clock pointer Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 08/25] clk: bcm: rpi: Make sure pllb_arm is removed Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 09/25] clk: bcm: rpi: Remove pllb_arm_lookup global pointer Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 10/25] clk: bcm: rpi: Switch to clk_hw_register_clkdev Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 11/25] clk: bcm: rpi: Make sure the clkdev lookup is removed Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 12/25] clk: bcm: rpi: Use CCF boundaries instead of rolling our own Maxime Ripard
2020-06-04 18:02 ` Nicolas Saenz Julienne
2020-06-05 9:28 ` Maxime Ripard
2020-06-05 9:34 ` Nicolas Saenz Julienne
2020-05-27 15:45 ` [PATCH v3 13/25] clk: bcm: rpi: Create a data structure for the clocks Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 14/25] clk: bcm: rpi: Add clock id to data Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 15/25] clk: bcm: rpi: Pass the clocks data to the firmware function Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 16/25] clk: bcm: rpi: Rename is_prepared function Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 17/25] clk: bcm: rpi: Split pllb clock hooks Maxime Ripard
2020-06-05 10:34 ` Nicolas Saenz Julienne
2020-05-27 15:45 ` [PATCH v3 18/25] clk: bcm: rpi: Make the PLLB registration function return a clk_hw Maxime Ripard
2020-06-05 10:38 ` Nicolas Saenz Julienne
2020-05-27 15:45 ` [PATCH v3 19/25] clk: bcm: rpi: Add DT provider for the clocks Maxime Ripard
2020-06-05 10:42 ` Nicolas Saenz Julienne
2020-05-27 15:45 ` [PATCH v3 20/25] clk: bcm: rpi: Add an enum for the firmware clocks Maxime Ripard
2020-06-05 12:04 ` Nicolas Saenz Julienne
2020-06-05 13:09 ` Nicolas Saenz Julienne
2020-05-27 15:45 ` [PATCH v3 21/25] clk: bcm: rpi: Discover " Maxime Ripard
2020-06-05 12:45 ` Nicolas Saenz Julienne [this message]
2020-05-27 15:45 ` [PATCH v3 22/25] clk: bcm: rpi: Give firmware clocks a name Maxime Ripard
2020-06-05 12:49 ` Nicolas Saenz Julienne
2020-05-27 15:45 ` [PATCH v3 23/25] Revert "clk: bcm2835: remove pllb" Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 24/25] clk: bcm: rpi: Remove the quirks for the CPU clock Maxime Ripard
2020-05-27 15:45 ` [PATCH v3 25/25] ARM: dts: bcm2711: Add firmware clocks node 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=a8741e98ce8ef4522def822e830de7d8b4a4604a.camel@suse.de \
--to=nsaenzjulienne@suse.de \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=dave.stevenson@raspberrypi.com \
--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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).