Hi Tomi,
On 4/8/2011 7:51 AM, Valkeinen, Tomi wrote:
> 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.
Both can be used for the system clock (sys_clk -> DSI DPLL or dss_dss_clk).
In fact after multiple discussions with DSS and PRCM folks, I realized that OMAP3 and 4 are using exactly the same clock scheme :-(
The confusion in my mind came from change in naming convention in both the PRCM and the DSS at the same time between OMAP3 and 4.
OMAP3 -> OMAP4
dss1_alwon_fck -> dss_dss_clk (from dpll per output in both cases)
dss2_alwon_fck -> dss_sys_clk
dss_ick -> dss_fck (-> modulemode) that name really needs to be changed to avoid further confusion.
So in fact the OMAP4 aliased should be:
CLK("omapdss_dss", "fck", &dss_dss_clk, CK_443X),
CLK("omapdss_dss", "sys_clk", &dss_sys_clk, CK_443X),
CLK("omapdss_dss", "ick", &dss_fck, CK_443X),
That will map perfectly what we have on OMAP3:
CLK("omapdss_dss", "fck", &dss1_alwon_fck_3430es2, ...),
CLK("omapdss_dss", "sys_clk", &dss2_alwon_fck, ...),
CLK("omapdss_dss", "ick", &dss_ick_3430es2, ...),
>> 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.
No indeed.
> 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.
For dedicated clock like tv_clk, it probably makes sense, because the other DSS drivers do not have to care about that clock.
> - 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?
Are you sure about that?
The dss_core does not have any reset in the sysonfig (only a status), but the dispc does have one and the dsi as well.
That being said, the dss modules have some issue with the softreset at boot time, so I'm not sure what these bits are really doing.
Regards,
Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
