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>

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
index 5c34fba5197565e0ba6d253fed214aabf100beea..5202862d89b5550d147491483c77e9f6f74e5cae 100644 (file)
--- 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)