Re: [Freedreno] [PATCH 1/5] clk: inherit clocks enabled by bootloader

2019-07-01 Thread Eric Anholt
Rob Clark  writes:

> From: Rob Clark 
>
> The goal here is to support inheriting a display setup by bootloader,
> although there may also be some non-display related use-cases.
>
> Rough idea is to add a flag for clks and power domains that might
> already be enabled when kernel starts, and which should not be
> disabled at late_initcall if the kernel thinks they are "unused".
>
> If bootloader is enabling display, and kernel is using efifb before
> real display driver is loaded (potentially from kernel module after
> userspace starts, in a typical distro kernel), we don't want to kill
> the clocks and power domains that are used by the display before
> userspace starts.
>
> Signed-off-by: Rob Clark 

Raspberry Pi is carrying downstream hacks to do similar stuff, and it
would be great to see CCF finally support this.


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 1/5] clk: inherit clocks enabled by bootloader

2019-07-01 Thread Jeffrey Hugo
On Sun, Jun 30, 2019 at 9:02 AM Rob Clark  wrote:
>
> From: Rob Clark 
>
> The goal here is to support inheriting a display setup by bootloader,
> although there may also be some non-display related use-cases.
>
> Rough idea is to add a flag for clks and power domains that might
> already be enabled when kernel starts, and which should not be
> disabled at late_initcall if the kernel thinks they are "unused".
>
> If bootloader is enabling display, and kernel is using efifb before
> real display driver is loaded (potentially from kernel module after
> userspace starts, in a typical distro kernel), we don't want to kill
> the clocks and power domains that are used by the display before
> userspace starts.
>
> Signed-off-by: Rob Clark 

Seems sane to me.  I'm curious what Stephen Boyd thinks.
I'll try to give it a spin on one of the 835 laptops.

Reviewed-by: Jeffrey Hugo 
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 1/5] clk: inherit clocks enabled by bootloader

2019-06-30 Thread Rob Clark
From: Rob Clark 

The goal here is to support inheriting a display setup by bootloader,
although there may also be some non-display related use-cases.

Rough idea is to add a flag for clks and power domains that might
already be enabled when kernel starts, and which should not be
disabled at late_initcall if the kernel thinks they are "unused".

If bootloader is enabling display, and kernel is using efifb before
real display driver is loaded (potentially from kernel module after
userspace starts, in a typical distro kernel), we don't want to kill
the clocks and power domains that are used by the display before
userspace starts.

Signed-off-by: Rob Clark 
---
 drivers/clk/clk.c| 48 
 drivers/clk/qcom/common.c| 25 +
 drivers/clk/qcom/dispcc-sdm845.c | 22 ---
 drivers/clk/qcom/gcc-sdm845.c|  3 +-
 include/linux/clk-provider.h | 10 +++
 5 files changed, 97 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index aa51756fd4d6..14460e87f508 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -66,6 +66,7 @@ struct clk_core {
unsigned long   flags;
boolorphan;
boolrpm_enabled;
+   boolinherit_enabled; /* clock was enabled by 
bootloader */
unsigned intenable_count;
unsigned intprepare_count;
unsigned intprotect_count;
@@ -1166,6 +1167,9 @@ static void clk_unprepare_unused_subtree(struct clk_core 
*core)
hlist_for_each_entry(child, >children, child_node)
clk_unprepare_unused_subtree(child);
 
+   if (core->inherit_enabled)
+   return;
+
if (core->prepare_count)
return;
 
@@ -1197,6 +1201,9 @@ static void clk_disable_unused_subtree(struct clk_core 
*core)
hlist_for_each_entry(child, >children, child_node)
clk_disable_unused_subtree(child);
 
+   if (core->inherit_enabled)
+   return;
+
if (core->flags & CLK_OPS_PARENT_ENABLE)
clk_core_prepare_enable(core->parent);
 
@@ -1270,6 +1277,45 @@ static int clk_disable_unused(void)
 }
 late_initcall_sync(clk_disable_unused);
 
+/* Ignore CLK_INHERIT_BOOTLOADER clocks enabled by bootloader.  This
+ * gives a debug knob to disable inheriting clks from bootloader, so
+ * that drivers that used to work, when loaded as a module, thanks
+ * to disabling "unused" clocks at late_initcall(), can continue to
+ * work.
+ *
+ * The proper solution is to fix the drivers.
+ */
+static bool clk_ignore_inherited;
+static int __init clk_ignore_inherited_setup(char *__unused)
+{
+   clk_ignore_inherited = true;
+   return 1;
+}
+__setup("clk_ignore_inherited", clk_ignore_inherited_setup);
+
+/* clock and it's parents are already prepared/enabled from bootloader,
+ * so simply record the fact.
+ */
+static void __clk_inherit_enabled(struct clk_core *core)
+{
+   unsigned long parent_rate = 0;
+   core->inherit_enabled = true;
+   if (core->parent) {
+   __clk_inherit_enabled(core->parent);
+   parent_rate = core->parent->rate;
+   }
+   if (core->ops->recalc_rate)
+   core->rate = core->ops->recalc_rate(core->hw, parent_rate);
+}
+
+void clk_inherit_enabled(struct clk *clk)
+{
+   if (clk_ignore_inherited)
+   return;
+   __clk_inherit_enabled(clk->core);
+}
+EXPORT_SYMBOL_GPL(clk_inherit_enabled);
+
 static int clk_core_determine_round_nolock(struct clk_core *core,
   struct clk_rate_request *req)
 {
@@ -3302,6 +3348,8 @@ static int __clk_core_init(struct clk_core *core)
 * are enabled during init but might not have a parent yet.
 */
if (parent) {
+   if (orphan->inherit_enabled)
+   __clk_inherit_enabled(parent);
/* update the clk tree topology */
__clk_set_parent_before(orphan, parent);
__clk_set_parent_after(orphan, parent, NULL);
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index a6b2f86112d8..0d542eeef9aa 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -290,6 +290,31 @@ int qcom_cc_really_probe(struct platform_device *pdev,
if (ret)
return ret;
 
+   /*
+* Check which of clocks that we inherit state from bootloader
+* are enabled, and fixup enable/prepare state (as well as that
+* of it's parents).
+*/
+   for (i = 0; i < num_clks; i++) {
+   struct clk_hw *hw;
+
+   if (!rclks[i])
+   continue;
+
+   hw = [i]->hw;
+
+   if (!(hw->init->flags & CLK_INHERIT_BOOTLOADER))
+   continue;
+
+