* [PATCHv2] drm/omap: dsi: Fix PM for display blank with paired dss_pll calls
@ 2019-02-07 15:45 Tony Lindgren
2019-02-08 9:11 ` Tomi Valkeinen
0 siblings, 1 reply; 3+ messages in thread
From: Tony Lindgren @ 2019-02-07 15:45 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, Laurent Pinchart, dri-devel, Sebastian Reichel
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>
---
Changes since v1:
- Updated with Tomi's suggested changes to not break DVI with
regulator_enable() made conditional for dsi_display_init_dsi()
- Updated comments to better describe what all goes wrong
---
drivers/gpu/drm/omapdrm/dss/dsi.c | 60 ++++++++++++++++---------------
1 file changed, 31 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
--- 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)
@@ -4096,11 +4082,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 ?
@@ -4108,6 +4094,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;
@@ -4136,10 +4130,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;
}
@@ -4158,7 +4155,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 int dsi_display_enable(struct omap_dss_device *dssdev)
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCHv2] drm/omap: dsi: Fix PM for display blank with paired dss_pll calls
2019-02-07 15:45 [PATCHv2] drm/omap: dsi: Fix PM for display blank with paired dss_pll calls Tony Lindgren
@ 2019-02-08 9:11 ` Tomi Valkeinen
2019-02-08 15:23 ` Tony Lindgren
0 siblings, 1 reply; 3+ messages in thread
From: Tomi Valkeinen @ 2019-02-08 9:11 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap, Laurent Pinchart, dri-devel, Sebastian Reichel
On 07/02/2019 17:45, Tony Lindgren wrote:
> 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>
> ---
>
> Changes since v1:
> - Updated with Tomi's suggested changes to not break DVI with
> regulator_enable() made conditional for dsi_display_init_dsi()
>
> - Updated comments to better describe what all goes wrong
>
> ---
> drivers/gpu/drm/omapdrm/dss/dsi.c | 60 ++++++++++++++++---------------
> 1 file changed, 31 insertions(+), 29 deletions(-)
Looks fine to me and works on panda. I'll queue this to the next merge
window (I presume no rush to get this into the current -rcs, it's a bit
late).
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCHv2] drm/omap: dsi: Fix PM for display blank with paired dss_pll calls
2019-02-08 9:11 ` Tomi Valkeinen
@ 2019-02-08 15:23 ` Tony Lindgren
0 siblings, 0 replies; 3+ messages in thread
From: Tony Lindgren @ 2019-02-08 15:23 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, Laurent Pinchart, dri-devel, Sebastian Reichel
* Tomi Valkeinen <tomi.valkeinen@ti.com> [190208 09:11]:
> Looks fine to me and works on panda. I'll queue this to the next merge
> window (I presume no rush to get this into the current -rcs, it's a bit
> late).
OK good to hear panda works too, my panda is in my rack..
This one has been broken for a long time and nobody noticed,
so next is just fine with me.
Regards,
Tony
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-08 15:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07 15:45 [PATCHv2] drm/omap: dsi: Fix PM for display blank with paired dss_pll calls Tony Lindgren
2019-02-08 9:11 ` Tomi Valkeinen
2019-02-08 15:23 ` Tony Lindgren
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.