From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Cousson, Benoit" Date: Mon, 06 Jun 2011 15:28:21 +0000 Subject: Re: [PATCH 19/27] OMAP: DSS2: Use PM runtime & HWMOD support Message-Id: <4DECF215.5020505@ti.com> List-Id: References: <1307095237-14805-1-git-send-email-tomi.valkeinen@ti.com> <1307095237-14805-20-git-send-email-tomi.valkeinen@ti.com> <871uzahnib.fsf@ti.com> <1307122985.2016.72.camel@deskari> <87hb86a5mm.fsf@ti.com> <1307174504.1777.24.camel@lappyti> <4DECCE90.6070201@ti.com> <1307365290.1910.39.camel@deskari> <4DECD2D7.6030207@ti.com> <1307366474.1910.44.camel@deskari> <4DECDA3A.7080808@ti.com> <1307368525.1910.50.camel@deskari> In-Reply-To: <1307368525.1910.50.camel@deskari> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: "Valkeinen, Tomi" Cc: "Hilman, Kevin" , "linux-omap@vger.kernel.org" , "linux-fbdev@vger.kernel.org" , "paul@pwsan.com" On 6/6/2011 3:55 PM, Valkeinen, Tomi wrote: > On Mon, 2011-06-06 at 15:46 +0200, Cousson, Benoit wrote: >> On 6/6/2011 3:21 PM, Valkeinen, Tomi wrote: >>> On Mon, 2011-06-06 at 15:15 +0200, Cousson, Benoit wrote: >>>> On 6/6/2011 3:01 PM, Valkeinen, Tomi wrote: >>>>> On Mon, 2011-06-06 at 14:56 +0200, Cousson, Benoit wrote: >>> >>>>> 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. >>> >>>> 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. >> >>> 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,