Re: [PATCH 19/27] OMAP: DSS2: Use PM runtime & HWMOD support

On 6/6/2011 3:55 PM, Valkeinen, Tomi wrote:

On Mon, 2011-06-06 at 15:46 +0200, Cousson, Benoit wrote:

quoted

On 6/6/2011 3:21 PM, Valkeinen, Tomi wrote:

quoted

On Mon, 2011-06-06 at 15:15 +0200, Cousson, Benoit wrote:

quoted

On 6/6/2011 3:01 PM, Valkeinen, Tomi wrote:

quoted

On Mon, 2011-06-06 at 14:56 +0200, Cousson, Benoit wrote:

quoted

quoted

In this long term solution, if the dss_fclk is the main_clk, how does
the framework handle the situation when we want to switch from the
standard DSS fclk to the one from DSI PLL?

That part cannot be done by the hwmod fmwk anyway. The goal of the fmwk
is to ensure that the module is accessible by the driver whatever the
PRCM clock used.
Enabling the DSI PLL will require the PRCM clock to be enabled first.

Using the DSI PLL as the fclk is doable, but is it really useful or needed?

Yes, it's useful and needed. It gives us much finer control to the clock
frequencies, and so allows us to go to higher frequencies and also more
exactly to the required pixel clock.

quoted

Assuming you need that mode, you will always have to explicitly switch
from DSI to PRCM clock before trying to disable the DSS.
This is something you will have to do inside the DSS driver. It should
be transparent to the hwmod fmwk.

This sounds ok.

I think the main question is how do we disable the standard DSS fclk
from PRCM when using DSI PLL? As far as I know, disabling that clock
will allow some areas of OMAP to be shut down even while DSS is working.
So from power management point of view it sounds a needed feature.

Yes, at least in theory, but considering that any use case that will
require the DSI PLL will use a LCD panel + backlight, or an OLED panel
that will consume 50 times more than the 186 MHz clock, I do not think
it is really needed.
Moreover, that clock is generated by the PER DPLL that will be always
enabled in most usecase because it does generate the UART, I2C and most
basic peripherals clocks. If we cannot gate the PER DPLL, there is no
saving to expect from gating the DSS fclk only.
Bottom-line is that there is no practical power saving to expect from
that mode.

quoted

If the clock is main_clk for the HWMOD, it sounds to me it's always
enabled if the HWMOD is enabled?

Yes, but that sounds to me a good trade off to avoid unnecessary
complexity in your driver or in the hwmod fmwk.

Ok, if there are no real power savings there, then I agree, it's
pointless to add that complexity.

So how do we go forward in short term? I'd very much like to remove all
the "silly" code from the DSS pm_runtime patch series caused by this
opt_clock handling. Is it possible to get some kind of a temporary
solution in the hwmod framework which would somehow solve this from DSS
driver's point of view? A flag that causes hwmod fmwk to enable
opt-clocks automatically? Or is it possible to have more than one
mandatory clock?

Before doing that, could you maybe just try something to make OMAP4 
looks a little bit more like OMAP3?

dss_fck -> ick
dss_dss_fck -> main_clk

That should ensure that both modulemode and the PRCM fclk will be 
managed by pm_runtime.

I just did a basic patch for the first module, you should maybe change 
some other entries.

Regards,
Benoit

---

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c 

b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 614d680..4dfd18a 100644

--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c

@@ -1134,7 +1134,7 @@ static struct omap_hwmod_addr_space 

omap44xx_dss_dma_addrs[] = {
  static struct omap_hwmod_ocp_if omap44xx_l3_main_2__dss = {
         .master         = &omap44xx_l3_main_2_hwmod,
         .slave          = &omap44xx_dss_hwmod,
-       .clk            = "l3_div_ck",
+       .clk            = "dss_fck",
         .addr           = omap44xx_dss_dma_addrs,
         .addr_cnt       = ARRAY_SIZE(omap44xx_dss_dma_addrs),
         .user           = OCP_USER_SDMA,

@@ -1167,14 +1167,13 @@ static struct omap_hwmod_ocp_if 

*omap44xx_dss_slaves[] = {
  static struct omap_hwmod_opt_clk dss_opt_clks[] = {
         { .role = "sys_clk", .clk = "dss_sys_clk" },
         { .role = "tv_clk", .clk = "dss_tv_clk" },
-       { .role = "dss_clk", .clk = "dss_dss_clk" },
         { .role = "video_clk", .clk = "dss_48mhz_clk" },
  };

  static struct omap_hwmod omap44xx_dss_hwmod = {
         .name           = "dss_core",
         .class          = &omap44xx_dss_hwmod_class,
-       .main_clk       = "dss_fck",
+       .main_clk       = "dss_dss_fck",
         .prcm           = {
                 .omap4 = {
                         .clkctrl_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,