On 7/14/2017 4:13 PM, Rob Clark wrote:
On Fri, Jul 14, 2017 at 12:52 AM, Rajendra Nayak <rnayak@xxxxxxxxxxxxxx> wrote:
Hi Rob,
On 07/11/2017 11:50 PM, Rob Clark wrote:
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 make corresponding fixups
to clk enable/prepare_count and power-domain state so that these are
not automatically disabled late in boot.
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.
Second part is for drm/msm to check if display related clocks are
enabled when it is loaded, and if so read back hw state to sync
existing display state w/ software state, and skip the initial
clk_enable's and otherwise fixing up clk/regulator/etc ref counts
(skipping the normal display-enable codepaths), therefore inheriting
the enable done by bootloader.
Obviously this should be split up into multiple patches and many
TODOs addressed. But I guess this is enough for now to start
discussing the approach, and in particular how drm and clock/pd
drivers work together to handle handover from bootloader.
The CLK_INHERIT_BOOTLOADER and related gsdc flag should only be set
on leaf nodes.
---
[]..
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index d523991c945f..90b698c910d0 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -11,6 +11,7 @@
* GNU General Public License for more details.
*/
+#include <linux/clk.h>
#include <linux/export.h>
#include <linux/module.h>
#include <linux/regmap.h>
@@ -258,6 +259,33 @@ 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).
+ *
+ * TODO can we assume that parents coming from another clk
+ * driver are already registered?
+ */
+ for (i = 0; i < num_clks; i++) {
+ struct clk_hw *hw;
+
+ if (!rclks[i])
+ continue;
+
+ hw = &rclks[i]->hw;
+
+ if (!(hw->init->flags & CLK_INHERIT_BOOTLOADER))
+ continue;
+
+ if (!clk_is_enabled_regmap(hw))
+ continue;
+
+ dev_dbg(dev, "%s is enabled from bootloader!\n",
+ hw->init->name);
+
+ clk_inherit_enabled(hw->clk);
how about just calling a clk_prepare_enable(hw->clk) instead of adding a new API?
The flag could also be something qcom specific and then we avoid having to add
anything in generic CCF code and its all handled in the qcom clock drivers.
Hmm, I could try that.. I *guess* it doesn't hurt to enable an
already enabled clk..
yes, ideally it shouldn't hurt.
beyond that, I wonder if this is something that other platforms would
want, so maybe I should be doing the check for enabled in CCF core?
If not, making this a qcom specific flag makes sense.
I think most previous attempts to solve this were done trying to keep
it very generic and they didn't go too far.
So I was thinking maybe we could deal with it within qcom drivers for
now, and if others come up with something similar, then look to
consolidate at a generic CCF level.
+ }
+
reset = &cc->reset;
reset->rcdev.of_node = dev->of_node;
reset->rcdev.ops = &qcom_reset_ops;
[] ..
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index a4f3580587b7..440d819b2d9d 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -291,6 +291,12 @@ static int gdsc_init(struct gdsc *sc)
if ((sc->flags & VOTABLE) && on)
gdsc_enable(&sc->pd);
+ if ((sc->flags & INHERIT_BL) && on) {
+ pr_debug("gdsc: %s is enabled from bootloader!\n", sc->pd.name);
+ gdsc_enable(&sc->pd);
+ sc->pd.flags |= GENPD_FLAG_ALWAYS_ON;
Would this not prevent the powerdomain from ever getting disabled?
Yeah, this is a hack, because I couldn't figure out something better.
The problem is that gdsc/powerdomain doesn't have it's own
enable_count but this is tracked at the device level (afaict.. maybe
I'm miss-understanding something). I guess we could add our own
enable_count within gdsc? Suggestions welcome, since I don't know
these parts of the code so well.
One way to deal with it would be to tell the genpd core, that the gdsc
isn't really enabled, so it does not go ahead and disable it thinking
its unused.
Once the display driver comes up, it runtime enables/disables it.
I am guessing, if the bootloader turns on the gdsc, there will also
be a kernel driver to handle it.
So I am basically saying,
if ((sc->flags & INHERIT_BL) && on)
on = !on; /* Prevent genpd from thinking its unsued */
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
