drm/omap: dsi: Fix PM for display blank with paired dss_pll calls

Currently dsi_display_init_dsi() calls dss_pll_enable() but it is not
paired with dss_pll_disable() in dsi_display_uninit_dsi(). This leaves
the DSS clocks enabled when the display is blanked wasting about extra
5mW of power while idle.

The clock that is left on by not calling dss_pll_disable() is
DSS_CLKCTRL bit 10 OPTFCLKEN_SYS_CLK that is the source clock for
DSI PLL.

We can fix this issue by by making the current dsi_pll_uninit() into
dsi_pll_disable(). This way we can just call dss_pll_disable() from
dsi_display_uninit_dsi() and the code becomes a bit easier to follow.

However, we need to also consider that DSI PLL can be muxed for DVI too
as pointed out by Tomi Valkeinen <tomi.valkeinen@ti.com>. In the DVI
case, we want to unconditionally disable the clocks. To get around this
issue, we separate out the DSI lane handling from dsi_pll_enable() and
dsi_pll_disable() as suggested by Tomi in an earlier experimental patch.

So we must only toggle the DSI regulator based on the vdds_dsi_enabled
flag from dsi_display_init_dsi() and dsi_display_uninit_dsi().

We need to make these two changes together to avoid breaking things
for DVI when fixing the DSI clock handling. And this all causes a
slight renumbering of the error path for dsi_display_init_dsi().

Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

1 files changed, 31 insertions, 29 deletions

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
index 5c34fba51975..5202862d89b5 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -1342,12 +1342,9 @@ static int dsi_pll_enable(struct dss_pll *pll)
          */
         dsi_enable_scp_clk(dsi);
 
-        if (!dsi->vdds_dsi_enabled) {
-                r = regulator_enable(dsi->vdds_dsi_reg);
-                if (r)
-                        goto err0;
-                dsi->vdds_dsi_enabled = true;
-        }
+        r = regulator_enable(dsi->vdds_dsi_reg);
+        if (r)
+                goto err0;
 
         /* XXX PLL does not come out of reset without this... */
         dispc_pck_free_enable(dsi->dss->dispc, 1);
@@ -1372,36 +1369,25 @@ static int dsi_pll_enable(struct dss_pll *pll)
 
         return 0;
 err1:
-        if (dsi->vdds_dsi_enabled) {
-                regulator_disable(dsi->vdds_dsi_reg);
-                dsi->vdds_dsi_enabled = false;
-        }
+        regulator_disable(dsi->vdds_dsi_reg);
 err0:
         dsi_disable_scp_clk(dsi);
         dsi_runtime_put(dsi);
         return r;
 }
 
-static void dsi_pll_uninit(struct dsi_data *dsi, bool disconnect_lanes)
+static void dsi_pll_disable(struct dss_pll *pll)
 {
+        struct dsi_data *dsi = container_of(pll, struct dsi_data, pll);
+
         dsi_pll_power(dsi, DSI_PLL_POWER_OFF);
-        if (disconnect_lanes) {
-                WARN_ON(!dsi->vdds_dsi_enabled);
-                regulator_disable(dsi->vdds_dsi_reg);
-                dsi->vdds_dsi_enabled = false;
-        }
+
+        regulator_disable(dsi->vdds_dsi_reg);
 
         dsi_disable_scp_clk(dsi);
         dsi_runtime_put(dsi);
 
-        DSSDBG("PLL uninit done\n");
-}
-
-static void dsi_pll_disable(struct dss_pll *pll)
-{
-        struct dsi_data *dsi = container_of(pll, struct dsi_data, pll);
-
-        dsi_pll_uninit(dsi, true);
+        DSSDBG("PLL disable done\n");
 }
 
 static int dsi_dump_dsi_clocks(struct seq_file *s, void *p)
@@ -4089,11 +4075,11 @@ static int dsi_display_init_dsi(struct dsi_data *dsi)
 
         r = dss_pll_enable(&dsi->pll);
         if (r)
-                goto err0;
+                return r;
 
         r = dsi_configure_dsi_clocks(dsi);
         if (r)
-                goto err1;
+                goto err0;
 
         dss_select_dsi_clk_source(dsi->dss, dsi->module_id,
                                   dsi->module_id == 0 ?
@@ -4101,6 +4087,14 @@ static int dsi_display_init_dsi(struct dsi_data *dsi)
 
         DSSDBG("PLL OK\n");
 
+        if (!dsi->vdds_dsi_enabled) {
+                r = regulator_enable(dsi->vdds_dsi_reg);
+                if (r)
+                        goto err1;
+
+                dsi->vdds_dsi_enabled = true;
+        }
+
         r = dsi_cio_init(dsi);
         if (r)
                 goto err2;
@@ -4129,10 +4123,13 @@ static int dsi_display_init_dsi(struct dsi_data *dsi)
 err3:
         dsi_cio_uninit(dsi);
 err2:
-        dss_select_dsi_clk_source(dsi->dss, dsi->module_id, DSS_CLK_SRC_FCK);
+        regulator_disable(dsi->vdds_dsi_reg);
+        dsi->vdds_dsi_enabled = false;
 err1:
-        dss_pll_disable(&dsi->pll);
+        dss_select_dsi_clk_source(dsi->dss, dsi->module_id, DSS_CLK_SRC_FCK);
 err0:
+        dss_pll_disable(&dsi->pll);
+
         return r;
 }
 
@@ -4151,7 +4148,12 @@ static void dsi_display_uninit_dsi(struct dsi_data *dsi, bool disconnect_lanes,
 
         dss_select_dsi_clk_source(dsi->dss, dsi->module_id, DSS_CLK_SRC_FCK);
         dsi_cio_uninit(dsi);
-        dsi_pll_uninit(dsi, disconnect_lanes);
+        dss_pll_disable(&dsi->pll);
+
+        if (disconnect_lanes) {
+                regulator_disable(dsi->vdds_dsi_reg);
+                dsi->vdds_dsi_enabled = false;
+        }
 }
 
 static void dsi_display_enable(struct omap_dss_device *dssdev)