All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v7 04/12] clk: Always clamp the rounded rate
@ 2022-04-07  7:56 ` Yassine Oudjana
  0 siblings, 0 replies; 10+ messages in thread
From: Yassine Oudjana @ 2022-04-07  7:56 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Yassine Oudjana, Mike Turquette, Stephen Boyd, Dave Stevenson,
	Phil Elwell, Tim Gover, Dom Cobley, dri-devel, linux-clk,
	linux-arm-msm

On Fri, 25 Feb 2022 15:35:26 +0100, Maxime Ripard <maxime@cerno.tech> wrote:
> The current core while setting the min and max rate properly in the
> clk_request structure will not make sure that the requested rate is
> within these boundaries, leaving it to each and every driver to make
> sure it is.
>
> It's not clear if this was on purpose or not, but this introduces some
> inconsistencies within the API.
>
> For example, a user setting a range and then calling clk_round_rate()
> with a value outside of that range will get the same value back
> (ignoring any driver adjustements), effectively ignoring the range that
> was just set.
>
> Another one, arguably worse, is that it also makes clk_round_rate() and
> clk_set_rate() behave differently if there's a range and the rate being
> used for both is outside that range. As we have seen, the rate will be
> returned unchanged by clk_round_rate(), but clk_set_rate() will error
> out returning -EINVAL.
>
> Let's make sure the framework will always clamp the rate to the current
> range found on the clock, which will fix both these inconsistencies.
>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/clk/clk.c      |  2 ++
>  drivers/clk/clk_test.c | 50 +++++++++++++++++++++++++++---------------
>  2 files changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 112911138a7b..6c4e10209568 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1348,6 +1348,8 @@ static int clk_core_determine_round_nolock(struct clk_core *core,
>  	if (!core)
>  		return 0;
>
> +	req->rate = clamp(req->rate, req->min_rate, req->max_rate);
> +

This breaks mmpll8_early on mmcc-msm8996, making GPU power up fail:

------------[ cut here ]------------
mmpll8_early failed to enable!
WARNING: CPU: 3 PID: 354 at drivers/clk/qcom/clk-alpha-pll.c:238 wait_for_pll+0xe8/0xfc
Modules linked in: q6asm_dai(+) ath10k_pci(+) q6afe_dai q6routing venus_enc panel_lgphilips_sw43101(+) venus_dec ath10k_core q6asm q6adm videobuf2_dma_contig qcom_camss snd_q6dsp_common snd_soc_wcd9335 q6afe snd_soc_apq8096 msm q6core ath regmap_slimbus v4l2_fwnode snd_soc_qcom_common apr snd_soc_core v4l2_async mac80211 venus_core videobuf2_dma_sg snd_compress v4l2_mem2mem videobuf2_memops videobuf2_v4l2 snd_pcm nxp_nci_i2c videobuf2_common libarc4 nxp_nci snd_timer gpu_sched videodev hci_uart drm_dp_helper snd i2c_qcom_cci nci btqca drm_kms_helper mc soundcore nfc syscopyarea sysfillrect bluetooth slim_qcom_ngd_ctrl sysimgblt pdr_interface fb_sys_fops slimbus lzo_rle qcom_spmi_haptics qcom_q6v5_mss qcom_spmi_temp_alarm qcom_q6v5_pas industrialio qcom_pil_info qcom_common qcom_q6v5 qcom_sysmon qmi_helpers mdt_loader socinfo pwm_ir_tx rmtfs_mem cfg80211 rfkill zram atmel_mxt_ts drm drm_panel_orientation_quirks ip_tables
CPU: 3 PID: 354 Comm: systemd-udevd Not tainted 5.18.0-rc1+ #361
Hardware name: Xiaomi Mi Note 2 (DT)
pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : wait_for_pll+0xe8/0xfc
lr : wait_for_pll+0xe8/0xfc
sp : ffff80000b01b250
x29: ffff80000b01b250 x28: ffff0000ae071960 x27: ffff000096a9ec00
x26: ffff0000801d7cf4 x25: 00000001cd074651 x24: 0000000000000000
x23: ffff800009129090 x22: ffff8000090d7c30 x21: 0000000000000000
x20: 0000000080000000 x19: ffff8000097b4598 x18: 0000000000000ca8
x17: 00000000000032a0 x16: 0000000000000007 x15: 00000000ffffffff
x14: 0000000000000001 x13: 0000000000000020 x12: fffffffffffcaebf
x11: ffff80000968afb0 x10: 000000000000000a x9 : ffff80000815bcc8
x8 : 000000000000000a x7 : 0000000000000006 x6 : 000000000000000c
x5 : ffff0000fdd16990 x4 : 0000000000000000 x3 : 0000000000000027
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000857e3b00
Call trace:
 wait_for_pll+0xe8/0xfc
 clk_alpha_pll_enable+0x118/0x150
 clk_core_enable+0x78/0x290
 clk_core_enable+0x58/0x290
 clk_core_enable+0x58/0x290
 clk_core_enable+0x58/0x290
 clk_enable+0x38/0x60
 clk_bulk_enable+0x48/0xb0
 msm_gpu_pm_resume+0xb4/0x20c [msm]
 a5xx_pm_resume+0x28/0x200 [msm]
 adreno_resume+0x2c/0x40 [msm]
 pm_generic_runtime_resume+0x38/0x50
 __genpd_runtime_resume+0x38/0x90
 genpd_runtime_resume+0xa4/0x2cc
 __rpm_callback+0x50/0x180
 rpm_callback+0x74/0x80
 rpm_resume+0x434/0x6b0
 __pm_runtime_resume+0x64/0xb0
 adreno_load_gpu+0x68/0x1b0 [msm]
 msm_open+0xf0/0x100 [msm]
 drm_file_alloc+0x150/0x234 [drm]
 drm_client_init+0xdc/0x180 [drm]
 drm_fb_helper_init+0x60/0x70 [drm_kms_helper]
 msm_fbdev_init+0x6c/0x100 [msm]
 msm_drm_bind+0x56c/0x640 [msm]
 try_to_bring_up_aggregate_device+0x230/0x320
 __component_add+0xac/0x194
 component_add+0x20/0x30
 dsi_dev_attach+0x2c/0x3c [msm]
 dsi_host_attach+0xa0/0x154 [msm]
 mipi_dsi_attach+0x34/0x50
 sw43101_probe+0x144/0x200 [panel_lgphilips_sw43101]
 mipi_dsi_drv_probe+0x2c/0x40
 really_probe+0x184/0x3d0
 __driver_probe_device+0x11c/0x190
 driver_probe_device+0x44/0xf4
 __driver_attach+0xd8/0x1f0
 bus_for_each_dev+0x7c/0xe0
 driver_attach+0x30/0x40
 bus_add_driver+0x150/0x230
 driver_register+0x84/0x140
 mipi_dsi_driver_register_full+0x64/0x70
 sw43101_driver_init+0x2c/0x1000 [panel_lgphilips_sw43101]
 do_one_initcall+0x50/0x2b0
 do_init_module+0x50/0x260
 load_module+0x229c/0x2ae0
 __do_sys_finit_module+0xac/0x12c
 __arm64_sys_finit_module+0x2c/0x3c
 invoke_syscall+0x50/0x120
 el0_svc_common.constprop.0+0xdc/0x100
 do_el0_svc+0x34/0xa0
 el0_svc+0x34/0xb0
 el0t_64_sync_handler+0xa4/0x130
 el0t_64_sync+0x18c/0x190
---[ end trace 0000000000000000 ]---
Failed to enable clk 'core': -110
msm 900000.mdss: [drm:adreno_load_gpu [msm]] *ERROR* Couldn't power up the GPU: -110



^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v7 00/12] clk: Improve clock range handling
@ 2022-02-25 14:35 Maxime Ripard
  2022-02-25 14:35   ` Maxime Ripard
  0 siblings, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2022-02-25 14:35 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd
  Cc: Dave Stevenson, Phil Elwell, Tim Gover, Dom Cobley, dri-devel,
	linux-clk, Maxime Ripard

Hi,

This is a follow-up of the discussion here:
https://lore.kernel.org/linux-clk/20210319150355.xzw7ikwdaga2dwhv@gilmour/

and here:
https://lore.kernel.org/all/20210914093515.260031-1-maxime@cerno.tech/

While the initial proposal implemented a new API to temporarily raise and lower
clock rates based on consumer workloads, Stephen suggested an
alternative approach implemented here.

The main issue that needed to be addressed in our case was that in a
situation where we would have multiple calls to clk_set_rate_range, we
would end up with a clock at the maximum of the minimums being set. This
would be expected, but the issue was that if one of the users was to
relax or drop its requirements, the rate would be left unchanged, even
though the ideal rate would have changed.

So something like

clk_set_rate(user1_clk, 1000);
clk_set_min_rate(user1_clk, 2000);
clk_set_min_rate(user2_clk, 3000);
clk_set_min_rate(user2_clk, 1000);

Would leave the clock running at 3000Hz, while the minimum would now be
2000Hz.

This was mostly due to the fact that the core only triggers a rate
change in clk_set_rate_range() if the current rate is outside of the
boundaries, but not if it's within the new boundaries.

That series changes that and will trigger a rate change on every call,
with the former rate being tried again. This way, providers have a
chance to follow whatever policy they see fit for a given clock each
time the boundaries change.

This series also implements some kunit tests, first to test a few rate
related functions in the CCF, and then extends it to make sure that
behaviour has some test coverage.

Let me know what you think
Maxime

Changes from v6:
  - Removed unused header
  - Used more fitting KUNIT macros
  - Enhanced some comments
  - Reworded a commit message

Changes from v5:
  - Changed clk_hw_create_clk for clk_hw_get_clk since the former isn't
    exported to modules.
  - Added fix for clk_hw_get_clk

Changes from v4:
  - Rename the test file
  - Move all the tests to the first patch, and fix them up as fixes are done
  - Improved the test conditions
  - Added more tests
  - Improved commit messages
  - Fixed a regression where two disjoints clock ranges would now be accepted

Changes from v3:
  - Renamed the test file and Kconfig option
  - Add option to .kunitconfig
  - Switch to kunit_kzalloc
  - Use KUNIT_EXPECT_* instead of KUNIT_ASSERT_* where relevant
  - Test directly relevant calls instead of going through a temporary variable
  - Switch to more precise KUNIT_ASSERT_* macros where relevant

Changes from v2:
  - Rebased on current next
  - Rewrote the whole thing according to Stephen reviews
  - Implemented some kunit tests

Changes from v1:
  - Return NULL in clk_request_start if clk pointer is NULL
  - Test for clk_req pointer in clk_request_done
  - Add another user in vc4
  - Rebased on top of v5.15-rc1

Maxime Ripard (12):
  clk: Fix clk_hw_get_clk() when dev is NULL
  clk: Introduce Kunit Tests for the framework
  clk: Enforce that disjoints limits are invalid
  clk: Always clamp the rounded rate
  clk: Use clamp instead of open-coding our own
  clk: Always set the rate on clk_set_range_rate
  clk: Add clk_drop_range
  clk: bcm: rpi: Add variant structure
  clk: bcm: rpi: Set a default minimum rate
  clk: bcm: rpi: Run some clocks at the minimum rate allowed
  drm/vc4: Add logging and comments
  drm/vc4: hdmi: Remove clock rate initialization

 drivers/clk/.kunitconfig          |   1 +
 drivers/clk/Kconfig               |   7 +
 drivers/clk/Makefile              |   1 +
 drivers/clk/bcm/clk-raspberrypi.c | 125 ++++-
 drivers/clk/clk.c                 |  76 ++-
 drivers/clk/clk_test.c            | 795 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/vc4/vc4_hdmi.c    |  13 -
 drivers/gpu/drm/vc4/vc4_kms.c     |  11 +
 include/linux/clk.h               |  11 +
 9 files changed, 985 insertions(+), 55 deletions(-)
 create mode 100644 drivers/clk/clk_test.c

-- 
2.35.1


^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-04-07 15:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-07  7:56 [PATCH v7 04/12] clk: Always clamp the rounded rate Yassine Oudjana
2022-04-07  7:56 ` Yassine Oudjana
2022-04-07  8:08 ` Maxime Ripard
2022-04-07  8:08   ` Maxime Ripard
2022-04-07  8:27   ` Yassine Oudjana
2022-04-07  8:27     ` Yassine Oudjana
2022-04-07 15:27     ` Maxime Ripard
2022-04-07 15:27       ` Maxime Ripard
  -- strict thread matches above, loose matches on Subject: below --
2022-02-25 14:35 [PATCH v7 00/12] clk: Improve clock range handling Maxime Ripard
2022-02-25 14:35 ` [PATCH v7 04/12] clk: Always clamp the rounded rate Maxime Ripard
2022-02-25 14:35   ` Maxime Ripard

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.