[RFC 0/3] drm/msm+clk: handover of bootloader display

[RFC 0/3] drm/msm+clk: handover of bootloader display

[Rob Clark]

So now that this is working (at least on a single device), I figured
it was a good time to send out an RFC to start discussion about how
to do this properly, in particular the CCF/powerdomain parts.

The first patch adds flags so we can mark power domains and leaf
node clocks which might (or might not) be enabled by bootloader and
which we want to inherit when real display driver is probed.  (There
might be use-cases outside of display.. such as debug serial port?
I guess display is the most common/complex/useful use-case.)  From
the CCF/genpd standpoint, "inherit" really just means "fixup enable/
prepare_count and don't automatically switch off in lateinit".  The
rest of the complexity is in the display driver.

For display, there are two different cases depending on whether the
display driver is built as a module (ie. probes after lateinit) or
not.  If the display driver is built as a module, then we want efifb
to keep working after the clk_disable_unused + genpd_power_off_unused
late_initcall's.  And in either case, we want the display driver to
be able to detect that display is already on (by checking whether
various clocks are already enabled) so that it knows to readback the
hw state.  (And not try to do things like clk_set_rate() on already
running clocks.)

Right now this is working on 8x16 (dragonboard 410c).  I'm not sure
if we'll hit more issues in CCF with more complex devices due to
separate GCC and MMCC clock controllers (a bit fuzzy on how things
work with initially unparented clocks, if for example MMCC probes
before it's GCC parent clocks.. and in general I am by no means an
expert about the common clock framework).  Certainly I'd like to
hear suggestions about how to procede on the CCF/genpd front.

I am aware of another similar RFC[1].  Although on most of the
snapdragon devices you can in theory enable/disable the bootloader
splashscreen via a 'fastboot oem select-display-panel' command, so
I think whatever solution we have needs to introspect the hw to
decide what to do.

The latest version of these patches (plus various cleanups/etc that
they depend on) can be found here[2]

[1] https://lkml.org/lkml/2017/6/28/188
[2] https://github.com/freedreno/kernel-msm/commits/display-handover

Rob Clark (3):
  clk: inherit display clocks enabled by bootloader
  drm/msm: inherit display from bootloader
  drm/bridge: adv7511: deal with bootloader lighting up display

 drivers/clk/clk.c                               |  18 ++++
 drivers/clk/qcom/common.c                       |  28 +++++
 drivers/clk/qcom/gcc-msm8916.c                  |  15 +--
 drivers/clk/qcom/gdsc.c                         |   6 ++
 drivers/clk/qcom/gdsc.h                         |   1 +
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c    |  57 ++++++----
 drivers/gpu/drm/bridge/adv7511/adv7533.c        |   3 +
 drivers/gpu/drm/msm/dsi/dsi.h                   |   1 +
 drivers/gpu/drm/msm/dsi/dsi_host.c              |  32 ++++++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c           |   5 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h           |   1 +
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c      |   2 +
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c      |   2 +
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c |   2 +
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c |   8 ++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c        |  21 ++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c         |  48 +++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h         |   3 +
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c     | 132 ++++++++++++++++++++++--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c         |  79 +++++++++++++-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h         |  11 ++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       |  79 ++++++++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c         |  45 ++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h         |   2 +
 drivers/gpu/drm/msm/msm_drv.c                   |   3 +
 drivers/gpu/drm/msm/msm_drv.h                   |   2 +
 drivers/gpu/drm/msm/msm_fbdev.c                 |  15 ++-
 drivers/gpu/drm/msm/msm_kms.h                   |   2 +
 include/linux/clk-provider.h                    |   1 +
 include/linux/clk.h                             |   9 ++
 30 files changed, 591 insertions(+), 42 deletions(-)

-- 
2.13.0

[RFC 1/3] clk: inherit display clocks enabled by bootloader

[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 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.
---
 drivers/clk/clk.c              | 18 ++++++++++++++++++
 drivers/clk/qcom/common.c      | 28 ++++++++++++++++++++++++++++
 drivers/clk/qcom/gcc-msm8916.c | 15 ++++++++-------
 drivers/clk/qcom/gdsc.c        |  6 ++++++
 drivers/clk/qcom/gdsc.h        |  1 +
 include/linux/clk-provider.h   |  1 +
 include/linux/clk.h            |  9 +++++++++
 7 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index fc58c52a26b4..7c84e8d6e816 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -796,6 +796,24 @@ static void clk_disable_unused_subtree(struct clk_core *core)
 		clk_core_disable_unprepare(core->parent);
 }
 
+/* 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)
+{
+	core->enable_count++;
+	core->prepare_count++;
+if (core->hw->init->name) // XXX hack.. something funny with xo/xo_board..
+	if (core->parent)
+		__clk_inherit_enabled(core->parent);
+}
+
+void clk_inherit_enabled(struct clk *clk)
+{
+	__clk_inherit_enabled(clk->core);
+}
+EXPORT_SYMBOL_GPL(clk_inherit_enabled);
+
 static bool clk_ignore_unused;
 static int __init clk_ignore_unused_setup(char *__unused)
 {
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);
+	}
+
 	reset = &cc->reset;
 	reset->rcdev.of_node = dev->of_node;
 	reset->rcdev.ops = &qcom_reset_ops;
diff --git a/drivers/clk/qcom/gcc-msm8916.c b/drivers/clk/qcom/gcc-msm8916.c
index 2cfe7000fc60..c939d33f6377 100644
--- a/drivers/clk/qcom/gcc-msm8916.c
+++ b/drivers/clk/qcom/gcc-msm8916.c
@@ -2468,7 +2468,7 @@ static struct clk_branch gcc_mdss_ahb_clk = {
 				"pcnoc_bfdcd_clk_src",
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT,
+			.flags = CLK_SET_RATE_PARENT | CLK_INHERIT_BOOTLOADER,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -2485,7 +2485,7 @@ static struct clk_branch gcc_mdss_axi_clk = {
 				"system_noc_bfdcd_clk_src",
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT,
+			.flags = CLK_SET_RATE_PARENT | CLK_INHERIT_BOOTLOADER,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -2502,7 +2502,7 @@ static struct clk_branch gcc_mdss_byte0_clk = {
 				"byte0_clk_src",
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT,
+			.flags = CLK_SET_RATE_PARENT | CLK_INHERIT_BOOTLOADER,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -2519,7 +2519,7 @@ static struct clk_branch gcc_mdss_esc0_clk = {
 				"esc0_clk_src",
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT,
+			.flags = CLK_SET_RATE_PARENT | CLK_INHERIT_BOOTLOADER,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -2536,7 +2536,7 @@ static struct clk_branch gcc_mdss_mdp_clk = {
 				"mdp_clk_src",
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT,
+			.flags = CLK_SET_RATE_PARENT | CLK_INHERIT_BOOTLOADER,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -2553,7 +2553,7 @@ static struct clk_branch gcc_mdss_pclk0_clk = {
 				"pclk0_clk_src",
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT,
+			.flags = CLK_SET_RATE_PARENT | CLK_INHERIT_BOOTLOADER,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -2570,7 +2570,7 @@ static struct clk_branch gcc_mdss_vsync_clk = {
 				"vsync_clk_src",
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT,
+			.flags = CLK_SET_RATE_PARENT | CLK_INHERIT_BOOTLOADER,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -3060,6 +3060,7 @@ static struct gdsc mdss_gdsc = {
 		.name = "mdss",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.flags = INHERIT_BL,
 };
 
 static struct gdsc jpeg_gdsc = {
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;
+	}
+
 	if (on || (sc->pwrsts & PWRSTS_RET))
 		gdsc_force_mem_on(sc);
 	else
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 39648348e5ec..3b5e64b060c2 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -53,6 +53,7 @@ struct gdsc {
 #define VOTABLE		BIT(0)
 #define CLAMP_IO	BIT(1)
 #define HW_CTRL		BIT(2)
+#define INHERIT_BL	BIT(3)
 	struct reset_controller_dev	*rcdev;
 	unsigned int			*resets;
 	unsigned int			reset_count;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index c59c62571e4f..4d5505f92329 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -35,6 +35,7 @@
 #define CLK_IS_CRITICAL		BIT(11) /* do not gate, ever */
 /* parents need enable during gate/ungate, set rate and re-parent */
 #define CLK_OPS_PARENT_ENABLE	BIT(12)
+#define CLK_INHERIT_BOOTLOADER	BIT(13) /* clk may be enabled from bootloader */
 
 struct clk;
 struct clk_hw;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 91bd464f4c9b..461991fc57e2 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -391,6 +391,15 @@ void clk_disable(struct clk *clk);
 void clk_bulk_disable(int num_clks, const struct clk_bulk_data *clks);
 
 /**
+ * clk_inherit_enabled - update the enable/prepare count of a clock and it's
+ * parents for clock enabled by bootloader.
+ *
+ * Intended to be used by clock drivers to inform the clk core of a clock
+ * that is already running.
+ */
+void clk_inherit_enabled(struct clk *clk);
+
+/**
  * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
  *		  This is only valid once the clock source has been enabled.
  * @clk: clock source
-- 
2.13.0

[RFC 2/3] drm/msm: inherit display from bootloader

[Rob Clark]

It is quite common for bootloader to enable display on tablets/phones.
But until now for upstream kernel we've been ignoring that since it
highly confuses drm/msm, and recommending to disable the bootloader
display.  (Otherwise we end up trying to set rates on already enabled
clks and all sorts of similar fun.)  But to get grub graphical boot
menu to work (and also efifb for early boot), we need to solve this
properly.

This could possibly be split out better.  But the basic idea is to

(a) determine what is enabled at probe time by looking at what clocks
    are enabled (ie. if display is enabled at all, mdp5's core clock
    will be on.  If DSI output is enabled (at least in video mode)
    the dsi block's pixel clock will be enabled, etc.

(b) For the enabled outputs, work backwards up the display pipe from
    output to plane (input).  We need to work in this direction as
    we cannot (for example) read back crtc state until we know which
    layer-mixer is used or read plane state until we know which hw
    pipe is used.  (But for mdp5 which drm_plane or drm_crtc is
    picked does not matter since they are virtualized, ie. we assign
    dynamically hwpipes and lm's to planes and crtcs.)

So far this is only implemented for DSI video mode.  For DSI command-
mode I think we end up needing to get the mode from the drm_panel.
(On db410c, which I'm testing this on, it is DSI video mode to an
adv7533 bridge.)  HDMI/eDP are unimplemented so far.

We have to make some simplifications/assumptions in a few places, about
how the bootloader is setting things up (ie. that there is just a single
plane, and about the pixel format).  I'm not sure that working backwards
from hw state is completely possible in all cases without making some
assumptions.  (For example, the hw is flexible enough to deal with
imaginary color formats like BARG5865.. and I think for 4k scanout
ganging up multiple hwpipes and layermixers you end up with multiple
possible sw states mapping to single hw state.)  Maybe there is some
room for a more generic optional framework for this (which can also be
used for extra debug to validate atomic commits via hw-readback after
commit), but probably best to start with some more simple display block
for that.
---
 drivers/gpu/drm/msm/dsi/dsi.h                   |   1 +
 drivers/gpu/drm/msm/dsi/dsi_host.c              |  32 ++++++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c           |   5 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h           |   1 +
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c      |   2 +
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c      |   2 +
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c |   2 +
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c |   8 ++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c        |  21 ++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c         |  48 +++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h         |   3 +
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c     | 132 ++++++++++++++++++++++--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c         |  79 +++++++++++++-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h         |  11 ++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       |  79 ++++++++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c         |  45 ++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h         |   2 +
 drivers/gpu/drm/msm/msm_drv.c                   |   3 +
 drivers/gpu/drm/msm/msm_drv.h                   |   2 +
 drivers/gpu/drm/msm/msm_fbdev.c                 |  15 ++-
 drivers/gpu/drm/msm/msm_kms.h                   |   2 +
 21 files changed, 478 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 9e6017387efb..4340edaec8e2 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -99,6 +99,7 @@ bool msm_dsi_manager_cmd_xfer_trigger(int id, u32 dma_base, u32 len);
 void msm_dsi_manager_attach_dsi_device(int id, u32 device_flags);
 int msm_dsi_manager_register(struct msm_dsi *msm_dsi);
 void msm_dsi_manager_unregister(struct msm_dsi *msm_dsi);
+void msm_dsi_hw_readback(struct msm_dsi *msm_dsi);
 
 /* msm dsi */
 static inline bool msm_dsi_device_connected(struct msm_dsi *msm_dsi)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 9e9c5696bc03..71a1d3dd7eb5 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -28,11 +28,14 @@
 #include <linux/regmap.h>
 #include <video/mipi_display.h>
 
+#include "msm_drv.h"
+#include "msm_kms.h"
 #include "dsi.h"
 #include "dsi.xml.h"
 #include "sfpb.xml.h"
 #include "dsi_cfg.h"
 #include "msm_kms.h"
+#include "phy/dsi_phy.h"
 
 static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
 {
@@ -1866,6 +1869,35 @@ void msm_dsi_host_unregister(struct mipi_dsi_host *host)
 	}
 }
 
+// XXX msm_dsi_host_hw_readback()... preserve the layer-cake?
+void msm_dsi_hw_readback(struct msm_dsi *msm_dsi)
+{
+	struct msm_dsi_host *msm_host = to_msm_dsi_host(msm_dsi->host);
+	struct msm_drm_private *priv = msm_dsi->dev->dev_private;
+	struct msm_kms *kms = priv->kms;
+
+	if (!__clk_is_enabled(msm_host->pixel_clk))
+		return;
+
+	kms->funcs->hw_readback_encoder(kms, msm_dsi->encoder);
+	msm_dsi->connector->state->crtc = msm_dsi->encoder->crtc;
+	msm_dsi->connector->state->best_encoder = msm_dsi->encoder;
+	msm_dsi->encoder->crtc->state->connector_mask =
+		(1 << drm_connector_index(msm_dsi->connector));
+	msm_host->power_on = true;
+
+	/* also fixup refcnt on regulators: */
+	dsi_host_regulator_enable(msm_host);
+
+	/* clocks will already be on, but with just a single refcnt,
+	 * whereas normally (at least in some cases) both dsi and
+	 * mdp5 take a reference to the same clks.  So take an extra
+	 * reference here to balance things out in the disable path.
+	 */
+	dsi_bus_clk_enable(msm_host);
+	dsi_phy_enable_resource(msm_dsi->phy);
+}
+
 int msm_dsi_host_xfer_prepare(struct mipi_dsi_host *host,
 				const struct mipi_dsi_msg *msg)
 {
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index 0c2eb9c9a1fc..ea0d674983dd 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -354,11 +354,13 @@ static int dsi_phy_regulator_enable(struct msm_dsi_phy *phy)
 	return ret;
 }
 
-static int dsi_phy_enable_resource(struct msm_dsi_phy *phy)
+int dsi_phy_enable_resource(struct msm_dsi_phy *phy)
 {
 	struct device *dev = &phy->pdev->dev;
 	int ret;
 
+	DBG("");
+
 	pm_runtime_get_sync(dev);
 
 	ret = clk_prepare_enable(phy->ahb_clk);
@@ -372,6 +374,7 @@ static int dsi_phy_enable_resource(struct msm_dsi_phy *phy)
 
 static void dsi_phy_disable_resource(struct msm_dsi_phy *phy)
 {
+	DBG("");
 	clk_disable_unprepare(phy->ahb_clk);
 	pm_runtime_put_sync(&phy->pdev->dev);
 }
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index 1733f6608a09..ee417c43c539 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -103,6 +103,7 @@ int msm_dsi_dphy_timing_calc_v2(struct msm_dsi_dphy_timing *timing,
 void msm_dsi_phy_set_src_pll(struct msm_dsi_phy *phy, int pll_id, u32 reg,
 				u32 bit_mask);
 int msm_dsi_phy_init_common(struct msm_dsi_phy *phy);
+int dsi_phy_enable_resource(struct msm_dsi_phy *phy);
 
 #endif /* __DSI_PHY_H__ */
 
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
index fe15aa64086f..fef3f208199a 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
@@ -1088,6 +1088,8 @@ struct msm_dsi_pll *msm_dsi_pll_14nm_init(struct platform_device *pdev, int id)
 	pll->save_state = dsi_pll_14nm_save_state;
 	pll->restore_state = dsi_pll_14nm_restore_state;
 	pll->set_usecase = dsi_pll_14nm_set_usecase;
+	pll->pll_on = pll_14nm_poll_for_ready(pll_14nm, POLL_MAX_READS,
+			POLL_TIMEOUT_US);
 
 	pll_14nm->vco_delay = 1;
 
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
index 26e3a01a99c2..f1634c533e1f 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
@@ -313,6 +313,7 @@ static const struct clk_ops clk_ops_dsi_pll_28nm_vco = {
 	.prepare = msm_dsi_pll_helper_clk_prepare,
 	.unprepare = msm_dsi_pll_helper_clk_unprepare,
 	.is_enabled = dsi_pll_28nm_clk_is_enabled,
+	.is_prepared = dsi_pll_28nm_clk_is_enabled,
 };
 
 /*
@@ -619,6 +620,7 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_init(struct platform_device *pdev,
 	pll->disable_seq = dsi_pll_28nm_disable_seq;
 	pll->save_state = dsi_pll_28nm_save_state;
 	pll->restore_state = dsi_pll_28nm_restore_state;
+	pll->pll_on = pll_28nm_poll_for_ready(pll_28nm, 10, 50);
 
 	if (type == MSM_DSI_PHY_28NM_HPM) {
 		pll_28nm->vco_delay = 1;
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
index 49008451085b..16eb49f22bf2 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
@@ -520,6 +520,8 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_8960_init(struct platform_device *pdev,
 	pll->disable_seq = dsi_pll_28nm_disable_seq;
 	pll->save_state = dsi_pll_28nm_save_state;
 	pll->restore_state = dsi_pll_28nm_restore_state;
+	pll->pll_on = pll_28nm_poll_for_ready(pll_28nm, POLL_MAX_READS,
+			POLL_TIMEOUT_US);
 
 	pll->en_seq_cnt = 1;
 	pll->enable_seqs[0] = dsi_pll_28nm_enable_seq;
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c
index aa7402e03f67..83322f52f971 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c
@@ -146,6 +146,14 @@ void mdp5_cmd_encoder_mode_set(struct drm_encoder *encoder,
 	mdp5_crtc_set_pipeline(encoder->crtc);
 }
 
+struct drm_display_mode *mdp5_cmd_encoder_readback_mode(struct drm_encoder *encoder)
+{
+	// TODO can we even reconstruct something that resembles a mode
+	// in the case of cmd mode?  (And will bootloader ever enable
+	// a panel in cmd mode, or can we just ignore this scenario?)
+	return NULL;
+}
+
 void mdp5_cmd_encoder_disable(struct drm_encoder *encoder)
 {
 	struct mdp5_encoder *mdp5_cmd_enc = to_mdp5_encoder(encoder);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
index 9d01656d853e..e0f8437ac868 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
@@ -409,6 +409,27 @@ static void mdp5_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	spin_unlock_irqrestore(&mdp5_crtc->lm_lock, flags);
 }
 
+void mdp5_crtc_readback(struct drm_crtc *crtc, struct drm_display_mode *mode,
+		enum mdp5_pipe pipe)
+{
+	struct drm_crtc_state *state = crtc->state;
+	struct drm_plane *plane = crtc->primary;
+	int ret;
+
+	// TODO probably just drop mdp5_state->enabled?
+	to_mdp5_crtc(crtc)->enabled = true;
+
+	state->active = true;
+	ret = drm_atomic_set_mode_for_crtc(state, mode);
+	WARN_ON(ret);
+	drm_mode_copy(&state->adjusted_mode, mode);
+
+	state->plane_mask = 1 << drm_plane_index(plane);
+	plane->state->crtc = crtc;
+
+	mdp5_plane_readback(plane, pipe);
+}
+
 static void mdp5_crtc_disable(struct drm_crtc *crtc)
 {
 	struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c
index 439e0a300e25..a4a27bfeaf31 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c
@@ -446,6 +446,54 @@ int mdp5_ctl_blend(struct mdp5_ctl *ctl, struct mdp5_pipeline *pipeline,
 	return 0;
 }
 
+/* Look at the CTL_LAYER_REG's and figure out currently used layermixer
+ * and pipe.  This is really only suitable for the single layer case,
+ * but that is all we'll get from the bootloader.
+ */
+struct mdp5_hw_mixer * mdp5_ctl_readback(struct mdp5_ctl *ctl,
+		enum mdp5_pipe *pipe)
+{
+	struct mdp5_kms *mdp5_kms = get_kms(ctl->ctlm);
+	int i;
+
+	for (i = 0; i < mdp5_kms->num_hwmixers; i++) {
+		struct mdp5_hw_mixer *mixer = mdp5_kms->hwmixers[i];
+		uint32_t reg;
+
+		reg = ctl_read(ctl, REG_MDP5_CTL_LAYER_REG(ctl->id, mixer->lm));
+
+		if (!reg)
+			continue;
+
+		if (FIELD(reg, MDP5_CTL_LAYER_REG_RGB0))
+			*pipe = SSPP_RGB0;
+		else if (FIELD(reg, MDP5_CTL_LAYER_REG_RGB1))
+			*pipe = SSPP_RGB1;
+		else if (FIELD(reg, MDP5_CTL_LAYER_REG_RGB2))
+			*pipe = SSPP_RGB2;
+		else if (FIELD(reg, MDP5_CTL_LAYER_REG_RGB3))
+			*pipe = SSPP_RGB3;
+		else if (FIELD(reg, MDP5_CTL_LAYER_REG_VIG0))
+			*pipe = SSPP_VIG0;
+		else if (FIELD(reg, MDP5_CTL_LAYER_REG_VIG1))
+			*pipe = SSPP_VIG1;
+		else if (FIELD(reg, MDP5_CTL_LAYER_REG_VIG2))
+			*pipe = SSPP_VIG2;
+		else if (FIELD(reg, MDP5_CTL_LAYER_REG_VIG2))
+			*pipe = SSPP_VIG2;
+		else if (FIELD(reg, MDP5_CTL_LAYER_REG_DMA0))
+			*pipe = SSPP_DMA0;
+		else if (FIELD(reg, MDP5_CTL_LAYER_REG_DMA1))
+			*pipe = SSPP_DMA1;
+		else
+			continue; /* WTF? */
+
+		return mixer;
+	}
+
+	return NULL;
+}
+
 u32 mdp_ctl_flush_mask_encoder(struct mdp5_interface *intf)
 {
 	if (intf->type == INTF_WB)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h
index b63120388dc6..5b8fa5df7713 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h
@@ -65,6 +65,9 @@ int mdp5_ctl_blend(struct mdp5_ctl *ctl, struct mdp5_pipeline *pipeline,
 		   enum mdp5_pipe r_stage[][MAX_PIPE_STAGE],
 		   u32 stage_cnt, u32 ctl_blend_op_flags);
 
+struct mdp5_hw_mixer * mdp5_ctl_readback(struct mdp5_ctl *ctl,
+		enum mdp5_pipe *pipe);
+
 /**
  * mdp_ctl_flush_mask...() - Register FLUSH masks
  *
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
index 97f3294fbfc6..b80b6cf19dc8 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
@@ -101,6 +101,18 @@ static const struct drm_encoder_funcs mdp5_encoder_funcs = {
 	.destroy = mdp5_encoder_destroy,
 };
 
+static void mdp5_encoder_setup_crtc_state(struct drm_encoder *encoder,
+		struct drm_crtc_state *crtc_state)
+{
+	struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
+	struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc_state);
+	struct mdp5_interface *intf = mdp5_encoder->intf;
+	struct mdp5_ctl *ctl = mdp5_encoder->ctl;
+
+	mdp5_cstate->ctl = ctl;
+	mdp5_cstate->pipeline.intf = intf;
+}
+
 static void mdp5_vid_encoder_mode_set(struct drm_encoder *encoder,
 				      struct drm_display_mode *mode,
 				      struct drm_display_mode *adjusted_mode)
@@ -209,6 +221,70 @@ static void mdp5_vid_encoder_mode_set(struct drm_encoder *encoder,
 	mdp5_crtc_set_pipeline(encoder->crtc);
 }
 
+static struct drm_display_mode *
+mdp5_vid_encoder_readback_mode(struct drm_encoder *encoder)
+{
+	struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
+	struct mdp5_kms *mdp5_kms = get_kms(encoder);
+	struct drm_display_mode *mode;
+	int intfn = mdp5_encoder->intf->num;
+	uint32_t hsync_ctl, display_hctl;
+	uint32_t vsync_period, vsync_len;
+	uint32_t display_v_start, display_v_end;
+	uint32_t dtv_hsync_skew = 0;  /* get this from panel? */
+
+	mode = drm_mode_create(encoder->dev);
+
+	hsync_ctl    = mdp5_read(mdp5_kms, REG_MDP5_INTF_HSYNC_CTL(intfn));
+	display_hctl = mdp5_read(mdp5_kms, REG_MDP5_INTF_DISPLAY_HCTL(intfn));
+	vsync_period = mdp5_read(mdp5_kms, REG_MDP5_INTF_VSYNC_PERIOD_F0(intfn));
+	display_v_start = mdp5_read(mdp5_kms, REG_MDP5_INTF_DISPLAY_VSTART_F0(intfn));
+	display_v_end   = mdp5_read(mdp5_kms, REG_MDP5_INTF_DISPLAY_VEND_F0(intfn));
+	vsync_len    = mdp5_read(mdp5_kms, REG_MDP5_INTF_VSYNC_LEN_F0(intfn));
+
+	// TODO I don't think there is a way to recover mode->clock??
+	mode->htotal      = FIELD(hsync_ctl, MDP5_INTF_HSYNC_CTL_PERIOD);
+	mode->hsync_start = mode->htotal - FIELD(display_hctl, MDP5_INTF_DISPLAY_HCTL_START);
+	mode->hsync_end   = FIELD(hsync_ctl, MDP5_INTF_HSYNC_CTL_PULSEW) + mode->hsync_start;
+	mode->hdisplay    = FIELD(display_hctl, MDP5_INTF_DISPLAY_HCTL_END) + 1 -
+			mode->htotal + mode->hsync_start;
+
+	if (mdp5_encoder->intf->type == INTF_eDP) {
+		display_v_start -= mode->htotal - mode->hsync_start;
+	}
+
+	mode->vtotal      = vsync_period / mode->htotal;
+	mode->vsync_start = mode->vtotal - ((display_v_start - dtv_hsync_skew) / mode->htotal);
+	mode->vsync_end   = (vsync_len / mode->htotal) + mode->vsync_start;
+	mode->vdisplay    = mode->vsync_start - ((vsync_period + dtv_hsync_skew - 1 - display_v_end) / mode->htotal);
+
+	// TODO maybe we want a flag to indicate that this is a readback
+	// mode, so not all fields are valid.  (Ie. when comparing to
+	// another mode to decide whether to do full modeset or not,
+	// ignore the fields that are zero.)
+	mode->type = DRM_MODE_TYPE_DRIVER;
+
+	// XXX how can we get these?  Maybe get modes from connector and
+	// see what fits (although that sounds ugly and annoying)
+	mode->clock = 148500;
+	mode->vrefresh = 60;
+
+	drm_mode_set_name(mode);
+
+	drm_mode_set_crtcinfo(mode, 0);
+
+	DBG("readback: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x",
+			mode->base.id, mode->name,
+			mode->vrefresh, mode->clock,
+			mode->hdisplay, mode->hsync_start,
+			mode->hsync_end, mode->htotal,
+			mode->vdisplay, mode->vsync_start,
+			mode->vsync_end, mode->vtotal,
+			mode->type, mode->flags);
+
+	return mode;
+}
+
 static void mdp5_vid_encoder_disable(struct drm_encoder *encoder)
 {
 	struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
@@ -282,6 +358,54 @@ static void mdp5_encoder_mode_set(struct drm_encoder *encoder,
 		mdp5_vid_encoder_mode_set(encoder, mode, adjusted_mode);
 }
 
+void mdp5_encoder_readback(struct drm_encoder *encoder)
+{
+	struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
+	struct msm_drm_private *priv = encoder->dev->dev_private;
+	struct mdp5_kms *mdp5_kms = get_kms(encoder);
+	struct drm_display_mode *mode;
+	struct mdp5_crtc_state *mdp5_cstate;
+	struct mdp5_hw_mixer *mixer;
+	enum mdp5_pipe pipe;
+
+	if (mdp5_encoder->intf->mode == MDP5_INTF_DSI_MODE_COMMAND)
+		mode = mdp5_cmd_encoder_readback_mode(encoder);
+	else
+		mode = mdp5_vid_encoder_readback_mode(encoder);
+
+	if (!mode)
+		return;
+
+	/* things like the chosen ctl and mixer need to be read back
+	 * and punched in to crtc state so that crtc knows what reg's
+	 * to readback.  The ctl is based on the encoder connected to
+	 * the crtc (ie. us) and the ctl is needed to figure out what
+	 * mixer is used.
+	 *
+	 * The good news is that since the crtc is fully virtualized,
+	 * we can just pick any one!
+	 */
+	encoder->crtc = priv->crtcs[0];
+	encoder->crtc->state->encoder_mask = (1 << drm_encoder_index(encoder));
+
+	mdp5_cstate = to_mdp5_crtc_state(encoder->crtc->state);
+
+	mdp5_encoder_setup_crtc_state(encoder, encoder->crtc->state);
+
+	mixer = mdp5_ctl_readback(mdp5_cstate->ctl, &pipe);
+	if (WARN_ON(!mixer))
+		return;
+
+	DBG("got mixer=%u, pipe=%s", mixer->lm, pipe2name(pipe));
+
+	mdp5_cstate->pipeline.mixer = mixer;
+	mdp5_kms->state->hwmixer.hwmixer_to_crtc[mixer->idx] = encoder->crtc;
+
+	mdp5_encoder->enabled = true;
+
+	mdp5_crtc_readback(encoder->crtc, mode, pipe);
+}
+
 static void mdp5_encoder_disable(struct drm_encoder *encoder)
 {
 	struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
@@ -308,13 +432,7 @@ static int mdp5_encoder_atomic_check(struct drm_encoder *encoder,
 				     struct drm_crtc_state *crtc_state,
 				     struct drm_connector_state *conn_state)
 {
-	struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
-	struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc_state);
-	struct mdp5_interface *intf = mdp5_encoder->intf;
-	struct mdp5_ctl *ctl = mdp5_encoder->ctl;
-
-	mdp5_cstate->ctl = ctl;
-	mdp5_cstate->pipeline.intf = intf;
+	mdp5_encoder_setup_crtc_state(encoder, crtc_state);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index d94c8a7657fa..12557ca9f2f0 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -22,6 +22,7 @@
 #include "msm_gem.h"
 #include "msm_mmu.h"
 #include "mdp5_kms.h"
+#include "dsi/dsi.h"
 
 static const char *iommu_ports[] = {
 		"mdp_0",
@@ -33,6 +34,9 @@ static int mdp5_hw_init(struct msm_kms *kms)
 	struct platform_device *pdev = mdp5_kms->pdev;
 	unsigned long flags;
 
+	if (mdp5_kms->poweron_enabled)
+		return 0;
+
 	pm_runtime_get_sync(&pdev->dev);
 	mdp5_enable(mdp5_kms);
 
@@ -72,6 +76,56 @@ static int mdp5_hw_init(struct msm_kms *kms)
 	return 0;
 }
 
+static void mdp5_hw_readback(struct msm_kms *kms)
+{
+	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+	struct msm_drm_private *priv = mdp5_kms->dev->dev_private;
+	unsigned i;
+
+	if (!mdp5_kms->poweron_enabled)
+		return;
+
+	/* We need to work backwards up the pipeline starting with the
+	 * connectors.
+	 *
+	 * TODO eDP
+	 * TODO HDMI
+	 */
+	for (i = 0; i < ARRAY_SIZE(priv->dsi); i++)
+		if (priv->dsi[i])
+			msm_dsi_hw_readback(priv->dsi[i]);
+
+	if (mdp5_kms->smp) {
+		struct drm_plane *plane;
+
+		/* readback SMP state for active planes: */
+		drm_for_each_plane(plane, mdp5_kms->dev) {
+			struct mdp5_plane_state *pstate =
+				to_mdp5_plane_state(plane->state);
+			if (!plane->state->crtc ||
+			    !plane->state->crtc->state->enable ||
+			    !pstate->hwpipe)
+				continue;
+			mdp5_smp_readback(mdp5_kms->smp,
+				&mdp5_kms->state->smp,
+				pstate->hwpipe->pipe);
+		}
+	}
+
+	if (drm_debug & DRM_UT_DRIVER) {
+		struct drm_printer p = drm_info_printer(mdp5_kms->dev->dev);
+		drm_state_dump(mdp5_kms->dev, &p);
+		if (mdp5_kms->smp)
+			mdp5_smp_dump(mdp5_kms->smp, &p);
+	}
+}
+
+static void mdp5_hw_readback_encoder(struct msm_kms *kms,
+		struct drm_encoder *encoder)
+{
+	mdp5_encoder_readback(encoder);
+}
+
 struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
 {
 	struct msm_drm_private *priv = s->dev->dev_private;
@@ -223,6 +277,8 @@ static int mdp5_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
 static const struct mdp_kms_funcs kms_funcs = {
 	.base = {
 		.hw_init         = mdp5_hw_init,
+		.hw_readback     = mdp5_hw_readback,
+		.hw_readback_encoder = mdp5_hw_readback_encoder,
 		.irq_preinstall  = mdp5_irq_preinstall,
 		.irq_postinstall = mdp5_irq_postinstall,
 		.irq_uninstall   = mdp5_irq_uninstall,
@@ -653,7 +709,9 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
 		if (mdp5_cfg_intf_is_virtual(config->hw->intf.connect[i]) ||
 		    !config->hw->intf.base[i])
 			continue;
-		mdp5_write(mdp5_kms, REG_MDP5_INTF_TIMING_ENGINE_EN(i), 0);
+
+		if (!mdp5_kms->poweron_enabled)
+			mdp5_write(mdp5_kms, REG_MDP5_INTF_TIMING_ENGINE_EN(i), 0);
 
 		mdp5_write(mdp5_kms, REG_MDP5_INTF_FRAME_LINE_COUNT_EN(i), 0x3);
 	}
@@ -908,11 +966,22 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
 	/* optional clocks: */
 	get_clk(pdev, &mdp5_kms->lut_clk, "lut_clk", false);
 
-	/* we need to set a default rate before enabling.  Set a safe
-	 * rate first, then figure out hw revision, and then set a
-	 * more optimal rate:
+	/* If clock is enabled when driver is loaded, then bootloader
+	 * has already set up the display:
 	 */
-	clk_set_rate(mdp5_kms->core_clk, 200000000);
+	if (__clk_is_enabled(mdp5_kms->core_clk)) {
+		mdp5_kms->poweron_enabled = true;
+		mdp5_kms->enable_count++;
+
+		/* let pm-runtime know that we are already enabled: */
+		pm_runtime_get_noresume(&pdev->dev);
+	} else {
+		/* we need to set a default rate before enabling.  Set a safe
+		 * rate first, then figure out hw revision, and then set a
+		 * more optimal rate:
+		 */
+		clk_set_rate(mdp5_kms->core_clk, 200000000);
+	}
 
 	pm_runtime_enable(&pdev->dev);
 	mdp5_kms->rpm_enabled = true;
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
index 3b6dd4250242..15d32357d58b 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
@@ -74,6 +74,7 @@ struct mdp5_kms {
 	spinlock_t resource_lock;
 
 	bool rpm_enabled;
+	bool poweron_enabled;     /* display already on when driver probed */
 
 	struct mdp_irq error_handler;
 
@@ -273,12 +274,15 @@ void mdp5_disable_vblank(struct msm_kms *kms, struct drm_crtc *crtc);
 int mdp5_irq_domain_init(struct mdp5_kms *mdp5_kms);
 void mdp5_irq_domain_fini(struct mdp5_kms *mdp5_kms);
 
+void mdp5_plane_readback(struct drm_plane *plane, enum mdp5_pipe pipe);
 uint32_t mdp5_plane_get_flush(struct drm_plane *plane);
 enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane);
 enum mdp5_pipe mdp5_plane_right_pipe(struct drm_plane *plane);
 struct drm_plane *mdp5_plane_init(struct drm_device *dev,
 				  enum drm_plane_type type);
 
+void mdp5_crtc_readback(struct drm_crtc *crtc, struct drm_display_mode *mode,
+		enum mdp5_pipe pipe);
 struct mdp5_ctl *mdp5_crtc_get_ctl(struct drm_crtc *crtc);
 uint32_t mdp5_crtc_vblank(struct drm_crtc *crtc);
 
@@ -290,6 +294,7 @@ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev,
 				struct drm_plane *plane,
 				struct drm_plane *cursor_plane, int id);
 
+void mdp5_encoder_readback(struct drm_encoder *encoder);
 struct drm_encoder *mdp5_encoder_init(struct drm_device *dev,
 		struct mdp5_interface *intf, struct mdp5_ctl *ctl);
 int mdp5_vid_encoder_set_split_display(struct drm_encoder *encoder,
@@ -302,6 +307,7 @@ u32 mdp5_encoder_get_framecount(struct drm_encoder *encoder);
 void mdp5_cmd_encoder_mode_set(struct drm_encoder *encoder,
 			       struct drm_display_mode *mode,
 			       struct drm_display_mode *adjusted_mode);
+struct drm_display_mode *mdp5_cmd_encoder_readback_mode(struct drm_encoder *encoder);
 void mdp5_cmd_encoder_disable(struct drm_encoder *encoder);
 void mdp5_cmd_encoder_enable(struct drm_encoder *encoder);
 int mdp5_cmd_encoder_set_split_display(struct drm_encoder *encoder,
@@ -312,6 +318,11 @@ static inline void mdp5_cmd_encoder_mode_set(struct drm_encoder *encoder,
 					     struct drm_display_mode *adjusted_mode)
 {
 }
+static inline struct drm_display_mode *
+mdp5_cmd_encoder_readback_mode(struct drm_encoder *encoder)
+{
+	return NULL;
+}
 static inline void mdp5_cmd_encoder_disable(struct drm_encoder *encoder)
 {
 }
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 1cbf8f823eba..933121274719 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -192,6 +192,85 @@ mdp5_plane_atomic_print_state(struct drm_printer *p,
 	drm_printf(p, "\tstage=%s\n", stage2name(pstate->stage));
 }
 
+void mdp5_plane_readback(struct drm_plane *plane, enum mdp5_pipe pipe)
+{
+	struct mdp5_kms *mdp5_kms = get_kms(plane);
+	struct drm_plane_state *state = plane->state;
+	struct mdp5_plane_state *mdp5_state = to_mdp5_plane_state(state);
+	struct msm_drm_private *priv = plane->dev->dev_private;
+	uint32_t reg, pitch, format;
+	unsigned i;
+
+	for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
+		struct mdp5_hw_pipe *hwpipe = mdp5_kms->hwpipes[i];
+
+		if (hwpipe->pipe == pipe) {
+			mdp5_state->hwpipe = hwpipe;
+			mdp5_kms->state->hwpipe.hwpipe_to_plane[hwpipe->idx] =
+					plane;
+			break;
+		}
+	}
+
+	if (WARN_ON(!mdp5_state->hwpipe))
+		return;
+
+	/* NOTE: we expect to just have a single layer, so punt on
+	 * bothering to figure out how to map blending state back
+	 * to plane state.  And assume no scaling or anything fancy.
+	 *
+	 * TODO: we can't really differentiate between two non-
+	 * overlaping planes hooked to one CRTC, and src-split
+	 * mode, can we?  For now just completely ignore r_hwpipe.
+	 */
+
+	reg = mdp5_read(mdp5_kms, REG_MDP5_PIPE_SRC_IMG_SIZE(pipe));
+	state->src_w = FIELD(reg, MDP5_PIPE_SRC_SIZE_WIDTH) << 16;
+	state->src_h = FIELD(reg, MDP5_PIPE_SRC_SIZE_HEIGHT) << 16;
+
+	reg = mdp5_read(mdp5_kms, REG_MDP5_PIPE_SRC_XY(pipe));
+	state->src_x = FIELD(reg, MDP5_PIPE_SRC_XY_X) << 16;
+	state->src_y = FIELD(reg, MDP5_PIPE_SRC_XY_Y) << 16;
+
+	reg = mdp5_read(mdp5_kms, REG_MDP5_PIPE_OUT_SIZE(pipe));
+	state->crtc_w = FIELD(reg, MDP5_PIPE_OUT_SIZE_WIDTH);
+	state->crtc_h = FIELD(reg, MDP5_PIPE_OUT_SIZE_HEIGHT);
+
+	reg = mdp5_read(mdp5_kms, REG_MDP5_PIPE_OUT_XY(pipe));
+	state->crtc_x = FIELD(reg, MDP5_PIPE_OUT_XY_X);
+	state->crtc_y = FIELD(reg, MDP5_PIPE_OUT_XY_Y);
+
+	state->visible = true;
+	mdp5_state->stage = STAGE_BASE;
+
+	/* Reconstruct a framebuffer.
+	 *
+	 * TODO: assume XRGB8888 .. unpatched lk probably actually
+	 * is using RGB565 or BGR888, but those also don't work with
+	 * grub / EFI GOP, so just ignore it for now instead of
+	 * figuring out how to map the format related registers back
+	 * to a fourcc.  Since we know lk is only going to use one
+	 * of 3 formats, perhaps we could take a simplified approach
+	 * by just looking at MDP5_PIPE_SRC_FORMAT.{A,R,G,B}_BPC.
+	 * Note that mdp5 hw is expressive enough to encoded crazy
+	 * formats (RXBG5856 anyone?) so I don't think we can really
+	 * do a perfect job at mapping back to drm fourcc in any
+	 * case.
+	 */
+	format = DRM_FORMAT_XRGB8888;
+
+	reg = mdp5_read(mdp5_kms, REG_MDP5_PIPE_SRC_STRIDE_A(pipe));
+	pitch = FIELD(reg, MDP5_PIPE_SRC_STRIDE_A_P0);
+
+	state->fb = msm_alloc_stolen_fb(plane->dev,
+			state->src_w >> 16,
+			state->src_h >> 16,
+			pitch, format);
+
+	priv->stolen_fb = state->fb;
+	drm_framebuffer_get(priv->stolen_fb);
+}
+
 static void mdp5_plane_reset(struct drm_plane *plane)
 {
 	struct mdp5_plane_state *mdp5_state;
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c
index 58f712d37e7f..86e9547d7bd4 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c
@@ -257,6 +257,35 @@ static unsigned update_smp_state(struct mdp5_smp *smp,
 	return nblks;
 }
 
+static void readback_smp_state(struct mdp5_smp *smp, u32 cid,
+		mdp5_smp_state_t *assigned)
+{
+	struct mdp5_kms *mdp5_kms = get_kms(smp);
+	u32 blk, reg, val;
+
+	for (blk = 0; blk < smp->blk_cnt; blk++) {
+		int idx = blk / 3;
+		int fld = blk % 3;
+
+		reg = mdp5_read(mdp5_kms, REG_MDP5_SMP_ALLOC_W_REG(idx));
+
+		switch (fld) {
+		case 0:
+			val = FIELD(reg, MDP5_SMP_ALLOC_W_REG_CLIENT0);
+			break;
+		case 1:
+			val = FIELD(reg, MDP5_SMP_ALLOC_W_REG_CLIENT1);
+			break;
+		case 2:
+			val = FIELD(reg, MDP5_SMP_ALLOC_W_REG_CLIENT2);
+			break;
+		}
+
+		if (val == cid)
+			set_bit(blk, *assigned);
+	}
+}
+
 void mdp5_smp_prepare_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state)
 {
 	enum mdp5_pipe pipe;
@@ -292,6 +321,22 @@ void mdp5_smp_complete_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state
 	state->released = 0;
 }
 
+void mdp5_smp_readback(struct mdp5_smp *smp, struct mdp5_smp_state *state,
+		enum mdp5_pipe pipe)
+{
+	unsigned i;
+
+	DBG("readback SMP state for %s", pipe2name(pipe));
+
+	for (i = 0; i < pipe2nclients(pipe); i++) {
+		u32 cid = pipe2client(pipe, i);
+		void *cs = state->client_state[cid];
+
+		readback_smp_state(smp, cid, cs);
+		bitmap_or(state->state, state->state, cs, smp->blk_cnt);
+	}
+}
+
 void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
 {
 	struct mdp5_kms *mdp5_kms = get_kms(smp);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h
index b41d0448fbe8..1fedeb6bc554 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h
@@ -94,5 +94,7 @@ void mdp5_smp_release(struct mdp5_smp *smp, struct mdp5_smp_state *state,
 
 void mdp5_smp_prepare_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state);
 void mdp5_smp_complete_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state);
+void mdp5_smp_readback(struct mdp5_smp *smp, struct mdp5_smp_state *state,
+		enum mdp5_pipe pipe);
 
 #endif /* __MDP5_SMP_H__ */
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 7a842b769925..542d8122a832 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -545,6 +545,9 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
 
 	drm_mode_config_reset(ddev);
 
+	if (kms->funcs->hw_readback)
+		kms->funcs->hw_readback(kms);
+
 #ifdef CONFIG_DRM_FBDEV_EMULATION
 	if (fbdev)
 		priv->fbdev = msm_fbdev_init(ddev);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index c6adf7aaea26..5ade6a0f467f 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -20,6 +20,7 @@
 
 #include <linux/kernel.h>
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/cpufreq.h>
 #include <linux/module.h>
 #include <linux/component.h>
@@ -109,6 +110,7 @@ struct msm_drm_private {
 	struct msm_file_private *lastctx;
 
 	struct drm_fb_helper *fbdev;
+	struct drm_framebuffer *stolen_fb;
 
 	struct msm_rd_state *rd;
 	struct msm_perf_state *perf;
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index bb068d7f979f..7d721fd4fc7a 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -78,7 +78,7 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
 	struct fb_info *fbi = NULL;
 	uint64_t paddr;
 	uint32_t format;
-	int ret, pitch;
+	int ret;
 
 	format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
 
@@ -86,9 +86,16 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
 			sizes->surface_height, sizes->surface_bpp,
 			sizes->fb_width, sizes->fb_height);
 
-	pitch = align_pitch(sizes->surface_width, sizes->surface_bpp);
-	fb = msm_alloc_stolen_fb(dev, sizes->surface_width,
-			sizes->surface_height, pitch, format);
+	if (priv->stolen_fb && (priv->stolen_fb->width == sizes->surface_width) &&
+	    (priv->stolen_fb->height == sizes->surface_height) &&
+	    (priv->stolen_fb->format->format == format)) {
+		DBG("Reusing stolen fb\n");
+		fb = priv->stolen_fb;
+	} else {
+		int pitch = align_pitch(sizes->surface_width, sizes->surface_bpp);
+		fb = msm_alloc_stolen_fb(dev, sizes->surface_width,
+				sizes->surface_height, pitch, format);
+	}
 
 	if (IS_ERR(fb)) {
 		dev_err(dev->dev, "failed to allocate fb\n");
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index a8f2ba5e5f07..0b20eece3827 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -33,6 +33,8 @@
 struct msm_kms_funcs {
 	/* hw initialization: */
 	int (*hw_init)(struct msm_kms *kms);
+	void (*hw_readback)(struct msm_kms *kms);
+	void (*hw_readback_encoder)(struct msm_kms *kms, struct drm_encoder *encoder);
 	/* irq handling: */
 	void (*irq_preinstall)(struct msm_kms *kms);
 	int (*irq_postinstall)(struct msm_kms *kms);
-- 
2.13.0

[RFC 3/3] drm/bridge: adv7511: deal with bootloader lighting up display

[Rob Clark]

We need to do some things a bit differently if the bridge chip is
already powered up when the driver loads (ie. bootloader has already
enabled display).  In particular we don't want to do anything that
would kill the display.
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 57 +++++++++++++++++++---------
 drivers/gpu/drm/bridge/adv7511/adv7533.c     |  3 ++
 2 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index f75ab6278113..15e725fa8616 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -107,6 +107,7 @@ static bool adv7511_register_volatile(struct device *dev, unsigned int reg)
 	case ADV7511_REG_BSTATUS(1):
 	case ADV7511_REG_CHIP_ID_HIGH:
 	case ADV7511_REG_CHIP_ID_LOW:
+	case ADV7511_REG_POWER:
 		return true;
 	}
 
@@ -584,6 +585,8 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
 			     edid_i2c_addr);
 	}
 
+	adv7511->current_edid_segment = -1;
+
 	edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511);
 
 	if (!adv7511->powered)
@@ -627,7 +630,8 @@ adv7511_detect(struct adv7511 *adv7511, struct drm_connector *connector)
 	 * has to be reinitialized. */
 	if (status == connector_status_connected && hpd && adv7511->powered) {
 		regcache_mark_dirty(adv7511->regmap);
-		adv7511_power_on(adv7511);
+		if (!adv7511->powered)
+			adv7511_power_on(adv7511);
 		adv7511_get_modes(adv7511, connector);
 		if (adv7511->status == connector_status_connected)
 			status = connector_status_disconnected;
@@ -1044,33 +1048,47 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 		return ret;
 	}
 
+	adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
+	if (IS_ERR(adv7511->regmap)) {
+		ret = PTR_ERR(adv7511->regmap);
+		goto uninit_regulators;
+	}
+
+	ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val);
+	if (ret)
+		goto uninit_regulators;
+	dev_dbg(dev, "Rev. %d\n", val);
+
+	ret = regmap_read(adv7511->regmap, ADV7511_REG_POWER, &val);
+	if (ret)
+		goto uninit_regulators;
+	dev_dbg(dev, "Power: %x\n", val);
+
+	if ((val & ADV7511_POWER_POWER_DOWN) == 0) {
+		dev_dbg(dev, "powered on at probe!\n");
+		regcache_sync(adv7511->regmap);
+		adv7511->powered = true;
+	}
+
 	/*
 	 * The power down GPIO is optional. If present, toggle it from active to
 	 * inactive to wake up the encoder.
 	 */
-	adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
+	adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd",
+			adv7511->powered ? GPIOD_ASIS : GPIOD_OUT_HIGH);
 	if (IS_ERR(adv7511->gpio_pd)) {
 		ret = PTR_ERR(adv7511->gpio_pd);
 		goto uninit_regulators;
 	}
 
-	if (adv7511->gpio_pd) {
+	if (adv7511->gpio_pd && !adv7511->powered) {
 		mdelay(5);
 		gpiod_set_value_cansleep(adv7511->gpio_pd, 0);
 	}
 
-	adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
-	if (IS_ERR(adv7511->regmap)) {
-		ret = PTR_ERR(adv7511->regmap);
-		goto uninit_regulators;
-	}
-
-	ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val);
-	if (ret)
-		goto uninit_regulators;
-	dev_dbg(dev, "Rev. %d\n", val);
-
-	if (adv7511->type == ADV7511)
+	if (adv7511->powered)
+		adv7511_hpd(adv7511);  /* clear pending irq's */
+	else if (adv7511->type == ADV7511)
 		ret = regmap_register_patch(adv7511->regmap,
 					    adv7511_fixed_registers,
 					    ARRAY_SIZE(adv7511_fixed_registers));
@@ -1085,7 +1103,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
 		     main_i2c_addr - 2);
 
-	adv7511_packet_disable(adv7511, 0xffff);
+	if (!adv7511->powered)
+		adv7511_packet_disable(adv7511, 0xffff);
 
 	adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
 	if (!adv7511->i2c_edid) {
@@ -1116,7 +1135,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 	regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL,
 		     ADV7511_CEC_CTRL_POWER_DOWN);
 
-	adv7511_power_off(adv7511);
+	if (!adv7511->powered)
+		adv7511_power_off(adv7511);
 
 	i2c_set_clientdata(i2c, adv7511);
 
@@ -1132,7 +1152,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 		goto err_unregister_cec;
 	}
 
-	adv7511_audio_init(dev, adv7511);
+	if (!adv7511->powered)
+		adv7511_audio_init(dev, adv7511);
 
 	return 0;
 
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index ac804f81e2f6..43dc53821acb 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -166,6 +166,9 @@ int adv7533_init_cec(struct adv7511 *adv)
 		goto err;
 	}
 
+	if (adv->powered)
+		return 0;
+
 	ret = regmap_register_patch(adv->regmap_cec,
 				    adv7533_cec_fixed_registers,
 				    ARRAY_SIZE(adv7533_cec_fixed_registers));
-- 
2.13.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 0/3] drm/msm+clk: handover of bootloader display

[Geert Uytterhoeven]

Hi Rob,

On Tue, Jul 11, 2017 at 8:20 PM, Rob Clark <robdclark@gmail.com> wrote:
> So now that this is working (at least on a single device), I figured
> it was a good time to send out an RFC to start discussion about how
> to do this properly, in particular the CCF/powerdomain parts.
>
> The first patch adds flags so we can mark power domains and leaf
> node clocks which might (or might not) be enabled by bootloader and
> which we want to inherit when real display driver is probed.  (There
> might be use-cases outside of display.. such as debug serial port?
> I guess display is the most common/complex/useful use-case.)  From
> the CCF/genpd standpoint, "inherit" really just means "fixup enable/
> prepare_count and don't automatically switch off in lateinit".  The
> rest of the complexity is in the display driver.
>
> For display, there are two different cases depending on whether the
> display driver is built as a module (ie. probes after lateinit) or
> not.  If the display driver is built as a module, then we want efifb
> to keep working after the clk_disable_unused + genpd_power_off_unused
> late_initcall's.  And in either case, we want the display driver to
> be able to detect that display is already on (by checking whether
> various clocks are already enabled) so that it knows to readback the
> hw state.  (And not try to do things like clk_set_rate() on already
> running clocks.)

For the PM domains: won't these just stay enabled if efifb would
call pm_runtime_enable() and pm_runtime_get_sync()?

For clocks, can't efifb enable all clocks tied to the device in DT?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC 0/3] drm/msm+clk: handover of bootloader display

[Rob Clark]

On Tue, Jul 11, 2017 at 2:35 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Rob,
>
> On Tue, Jul 11, 2017 at 8:20 PM, Rob Clark <robdclark@gmail.com> wrote:
>> So now that this is working (at least on a single device), I figured
>> it was a good time to send out an RFC to start discussion about how
>> to do this properly, in particular the CCF/powerdomain parts.
>>
>> The first patch adds flags so we can mark power domains and leaf
>> node clocks which might (or might not) be enabled by bootloader and
>> which we want to inherit when real display driver is probed.  (There
>> might be use-cases outside of display.. such as debug serial port?
>> I guess display is the most common/complex/useful use-case.)  From
>> the CCF/genpd standpoint, "inherit" really just means "fixup enable/
>> prepare_count and don't automatically switch off in lateinit".  The
>> rest of the complexity is in the display driver.
>>
>> For display, there are two different cases depending on whether the
>> display driver is built as a module (ie. probes after lateinit) or
>> not.  If the display driver is built as a module, then we want efifb
>> to keep working after the clk_disable_unused + genpd_power_off_unused
>> late_initcall's.  And in either case, we want the display driver to
>> be able to detect that display is already on (by checking whether
>> various clocks are already enabled) so that it knows to readback the
>> hw state.  (And not try to do things like clk_set_rate() on already
>> running clocks.)
>
> For the PM domains: won't these just stay enabled if efifb would
> call pm_runtime_enable() and pm_runtime_get_sync()?
>
> For clocks, can't efifb enable all clocks tied to the device in DT?

There is no node in dt (nor dt bindings) for efifb, and there really
should not be any, so no.

That could maybe work in the non-efi-boot case with simplefb except
that (a) we'd still need to know if clocks were already enabled at
boot or not, and in general having a dependency on clocks would keep
simplefb from probing anywhere near as early in boot, and (b) since
bootloader is populating simple-fb node in this case, it would end up
being kernel specific (ie. clk/powerdomain bindings work differently
between upstream and vendor kernel, so what the bootloader did would
end up being the right thing for downstream kernel and not upstream
kernel).  So in the end trying to solve this through DT doesn't really
accomplish anything.

BR,
-R
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 1/3] clk: inherit display clocks enabled by bootloader

[Rajendra Nayak]

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.

> +	}
> +
>  	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?

regards,
Rajendra

> +	}
> +
>  	if (on || (sc->pwrsts & PWRSTS_RET))
>  		gdsc_force_mem_on(sc);
>  	else
> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> index 39648348e5ec..3b5e64b060c2 100644
> --- a/drivers/clk/qcom/gdsc.h
> +++ b/drivers/clk/qcom/gdsc.h
> @@ -53,6 +53,7 @@ struct gdsc {
>  #define VOTABLE		BIT(0)
>  #define CLAMP_IO	BIT(1)
>  #define HW_CTRL		BIT(2)
> +#define INHERIT_BL	BIT(3)
>  	struct reset_controller_dev	*rcdev;
>  	unsigned int			*resets;
>  	unsigned int			reset_count;
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index c59c62571e4f..4d5505f92329 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -35,6 +35,7 @@
>  #define CLK_IS_CRITICAL		BIT(11) /* do not gate, ever */
>  /* parents need enable during gate/ungate, set rate and re-parent */
>  #define CLK_OPS_PARENT_ENABLE	BIT(12)
> +#define CLK_INHERIT_BOOTLOADER	BIT(13) /* clk may be enabled from bootloader */
>  
>  struct clk;
>  struct clk_hw;
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 91bd464f4c9b..461991fc57e2 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -391,6 +391,15 @@ void clk_disable(struct clk *clk);
>  void clk_bulk_disable(int num_clks, const struct clk_bulk_data *clks);
>  
>  /**
> + * clk_inherit_enabled - update the enable/prepare count of a clock and it's
> + * parents for clock enabled by bootloader.
> + *
> + * Intended to be used by clock drivers to inform the clk core of a clock
> + * that is already running.
> + */
> +void clk_inherit_enabled(struct clk *clk);
> +
> +/**
>   * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
>   *		  This is only valid once the clock source has been enabled.
>   * @clk: clock source
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [RFC 1/3] clk: inherit display clocks enabled by bootloader

[Rob Clark]

On Fri, Jul 14, 2017 at 12:52 AM, Rajendra Nayak <rnayak@codeaurora.org> 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..

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.

>> +     }
>> +
>>       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.

BR,
-R

> regards,
> Rajendra
>
>> +     }
>> +
>>       if (on || (sc->pwrsts & PWRSTS_RET))
>>               gdsc_force_mem_on(sc);
>>       else
>> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
>> index 39648348e5ec..3b5e64b060c2 100644
>> --- a/drivers/clk/qcom/gdsc.h
>> +++ b/drivers/clk/qcom/gdsc.h
>> @@ -53,6 +53,7 @@ struct gdsc {
>>  #define VOTABLE              BIT(0)
>>  #define CLAMP_IO     BIT(1)
>>  #define HW_CTRL              BIT(2)
>> +#define INHERIT_BL   BIT(3)
>>       struct reset_controller_dev     *rcdev;
>>       unsigned int                    *resets;
>>       unsigned int                    reset_count;
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index c59c62571e4f..4d5505f92329 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -35,6 +35,7 @@
>>  #define CLK_IS_CRITICAL              BIT(11) /* do not gate, ever */
>>  /* parents need enable during gate/ungate, set rate and re-parent */
>>  #define CLK_OPS_PARENT_ENABLE        BIT(12)
>> +#define CLK_INHERIT_BOOTLOADER       BIT(13) /* clk may be enabled from bootloader */
>>
>>  struct clk;
>>  struct clk_hw;
>> diff --git a/include/linux/clk.h b/include/linux/clk.h
>> index 91bd464f4c9b..461991fc57e2 100644
>> --- a/include/linux/clk.h
>> +++ b/include/linux/clk.h
>> @@ -391,6 +391,15 @@ void clk_disable(struct clk *clk);
>>  void clk_bulk_disable(int num_clks, const struct clk_bulk_data *clks);
>>
>>  /**
>> + * clk_inherit_enabled - update the enable/prepare count of a clock and it's
>> + * parents for clock enabled by bootloader.
>> + *
>> + * Intended to be used by clock drivers to inform the clk core of a clock
>> + * that is already running.
>> + */
>> +void clk_inherit_enabled(struct clk *clk);
>> +
>> +/**
>>   * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
>>   *             This is only valid once the clock source has been enabled.
>>   * @clk: clock source
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation

Re: [RFC 1/3] clk: inherit display clocks enabled by bootloader

[Nayak, Rajendra]

On 7/14/2017 4:13 PM, Rob Clark wrote:
> On Fri, Jul 14, 2017 at 12:52 AM, Rajendra Nayak <rnayak@codeaurora.org> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC 1/3] clk: inherit display clocks enabled by bootloader

[Stephen Boyd]

On 07/17, Nayak, Rajendra wrote:
> 
> 
> On 7/14/2017 4:13 PM, Rob Clark wrote:
> >On Fri, Jul 14, 2017 at 12:52 AM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
> >>Hi Rob,
> >>
> >>On 07/11/2017 11:50 PM, Rob Clark wrote:
> >>
> >>>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.

It hurts when you enable a PLL when it's already enabled. I
recall having to add code specifically for this reason to avoid
enabling a PLL that is already enabled out of the bootloader. But
that's because the enable sequence for a PLL is multiple writes
that first keep the output off and then turn it back on after
things stabilize. For RCGs this isn't the case.

> 
> >
> >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.

We know that other SoC vendors need handoff features as well, so
there will definitely be a point where we need to consolidate at
the framework level. Working something up that works for qcom
would be good to try out though to find edge cases, etc.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project