[PATCH 0/3] clk: Add __clk_disable_unprepare_counts_only() and use this in simple[fb|drm]

[Hans de Goede <johannes.goede@oss.qualcomm.com>]

Hi Michael, Stephen, et al.,

The device-tree simple-framebuffer device is special in that it does not
describe actual hardware, but it allows firmware (e.g. u-boot) to pass a
firmware initialized framebuffer to the OS to use as display during boot.

To avoid clocks used by the firmware framebuffer getting touched while in
use, the DT node contains a list of clocks which should not be touched
while the simple-framebuffer is in use.

Conceptually this is fine, in practice the *do not touch* is implemented as
a clk_prepare_enable() + clk_disable_unprepare() pair, with the disabling
happening when the simple-framebuffer driver gets replaced by the real
display-engine driver.

The simple-framebuffer concept assumes that the order in which resources
are acquired does not matter since they are already on, so power-sequencing
is not an issue. But clk_disable_unprepare() actually turns the clock off,
messing with the hardware state before the real display driver loads,
without any knowledge of the proper power-off sequence for the display.

The turning off of the clocks outside of a proper power-off sequence causes
the following error when the msm display driver tries to re-enable them:

[    2.980181] disp_cc_mdss_dptx3_pixel0_clk_src: rcg didn't update its configuration.
[    2.980272] WARNING: drivers/clk/qcom/clk-rcg2.c:136 at update_config+0xdc/0x100

Resulting in a non working display.

To fix this, we need a way to properly implement the "do not touch" concept
for clocks. clk_prepare_enable() is fine, the clocks are already on so no
ordering worries and it ensures that the clocks and their parents cannot
get turned off by incrementing their enable, prepare and protect counts.

clk_disable_unprepare() is a problem though since it actually turns the
clocks off. Instead something is needed which only decrements those counts.

This series introduces a new clk-core function called
__clk_disable_unprepare_counts_only() (1) which does just that. Prefixed
with '__' to indicate that normally drivers should not use this.

Michael, Stephen sorry for needing to add a new clk-core function for this,
but I see no other way of solving this (2)(3). The changes are not that
big / not too bad.

I've also considered making __clk_disable_unprepare_counts_only() implement
all the logic itself instead of adding the extra parameter to
clk_core_unprepare() and clk_core_disable() but that leads in duplicating
quite a bit of logic (4) so this seems better.

The other 2 patches just replace the clk_disable_unprepare() calls in
the simple[fb|drm] driver with the new helper.

This series fixes the display not coming up after switching to the msm
driver when a simple-framebuffer node with clocks listed is used.

Regards,

Hans


1) I'm open to changing the name, this is the best I could come up with.

2) One option considered was detaching the simple-framebuffer driver later,
   after the real display driver has had a chance to claim the clocks. But
   this won't work in cases where the real display driver picks different
   parent clocks then the boot firmware did and needs to reparent clocks.

   Basically the goal is for things to behave as if the simple-framebuffer
   driver was not there at all, because that leaves the hw in the state
   the real display driver expects.

3) There is precedence for something like this in other places in the
   kernel, e.g. pm_runtime_put_noidle() and the power_off parameter
   to dev_pm_domain_detach()

4) With the risk of the 2 copies of that logic diverging over time.



Hans de Goede (3):
  clk: Add __clk_disable_unprepare_counts_only() helper
  drm/sysfb: simpledrm: Use __clk_disable_unprepare_counts_only()
  fbdev: simplefb: Use __clk_disable_unprepare_counts_only()

 drivers/clk/clk.c                 | 41 ++++++++++++++++++++++---------
 drivers/gpu/drm/sysfb/simpledrm.c |  4 +--
 drivers/video/fbdev/simplefb.c    |  2 +-
 include/linux/clk.h               | 14 +++++++++++
 4 files changed, 46 insertions(+), 15 deletions(-)

-- 
2.54.0