Hi Paul, On Thu, 2011-04-07 at 13:27 -0600, Paul Walmsley wrote: > Hello Tomi, Benoît, > > On Mon, 4 Apr 2011, Tomi Valkeinen wrote: > > > On Fri, 2011-04-01 at 20:12 -0600, Paul Walmsley wrote: > > > > > Based on the E-mail thread so far, I'd say that changing the clock > > > aliases > > > is the way to go for right now. The clock aliases are not hardware data; > > > they just control how the clock hardware is mapped to the drivers. > > > > I'd very much prefer this option. Below is a patch for this. > > > > If Benoit doesn't complain too much about this, I'd like to get this > > merged as soon as possible, as OMAP4 DSS is currently crashing in the > > mainline kernel. > > I will queue your clock aliases patch and attempt to merge it upstream for > the -rc series. I am not happy to be disagreeing with Benoît here, but > this is the rationale that I am using. (Benoît, Tomi, please feel free to > correct if there is something blatantly false below.) > > 1. The integration of the DSS module is an unusual case. The > MODULEMODE bits for the DSS bits do not control the DSS "main" > functional clock (the clock that is needed to access device > registers); instead, they only control the DSS interface clock. > (This is because the DSS can use a functional clock that is not > provided by the OMAP core.) So calling the DSS MODULEMODE struct > clk "dss_fck" is not really correct, since it is really controlling > the interface clock.
If we don't apply the patch, I think the clock aliases are still broken. Let's forget the ick/fck thing for a second, and just think the clock from PRCM which is used as the source clock for pixel clock. That is currently called "fck" on OMAP2/3, but "dss_dss_clk" on OMAP4. This cannot be right, can it? From DSS's point of view that is the same clock, it should clearly have the same alias on all platforms. I don't care what the name is as long as it's consistent on all platforms. And I have to say I still don't quite get it what is the problem with "fck" name. Or is that meant to be a clock which enables the registers on the module, and not the clock used for the pixel clock? If so, I'm fine with having "fck" to be what it is currently, but then we need a new name for the clock used for pixel clock, which is consistent on all platforms. > 2. This patch does not create a precedent for hacking around general > driver clocking problems in the clock or clock data. This is only > needed because the MODULEMODE bits don't control the module > functional clock in this case. As I understand it, the only other > device that this problem could occur with is McBSP, due to > mcbsp_clks. > > 3. The existing DSS2 driver clock design apparently works fine when > this change is implemented[1], so no driver changes will be needed. > > 4. The proposed DSS driver patch to work around this uses a nasty hack > that really should be NACK'ed[2]. > > All this said, we may not be able to merge this change upstream, due to > the recent unhappiness about the clock data changes. If Linus rejects it, > you'll need a "Plan B." > > ... > > Also, I hope you and the other DSS hackers can finish the PM runtime > conversion of the DSS driver soon. Ideally before any new DSS > features are added. We really need to be able to separate the OMAP > integration details from the drivers, and right now, hwmod and PM > runtime are the best way we have to do that. I also started to look at the PM runtime, but it doesn't fix the issue with the inconsistent clock name I described above. I also have some questions regarding PM runtime, perhaps I'll just put them here as they are slightly related: - Should every DSS module handle their clocks independently, i.e. should VENC get its own clocks and DSI should get its own? If so, we need a bunch of new clock aliases so the devices can get their clocks. - Should every DSS module have their own PM runtime handling? (actually related to the question above) - If the modules are handled separately, how should the dependencies be handled? For example, dss_core's reset will reset all the other modules also, and most of the submodules need functions from dss_core and dss_dispc. So should, say, dss_dsi then call functions in core and dispc to "get" them, i.e. increase their pm runtime use count? - There seems to be some child/parent relationships in PM runtime. Should dss_core be the parent for the rest of the DSS modules? This would at least solve the reset issue easily, I guess. - How does saving/restoring the registers for OFF mode go with PM runtime? Should the registers be saved in runtime_suspend(), and restored in runtime_resume()? Can/should omap_pm_get_dev_context_loss_count() still be used to optimize the restoring? Tomi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
