Re: [PATCH v2 5/5] OMAPDSS: HDMI: Add support to dump clocks through

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

On Fri, 2011-09-23 at 12:10 +0530, K, Mythri P wrote:
> Hi,
> 
> On Fri, Sep 23, 2011 at 11:31 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Fri, 2011-09-23 at 11:22 +0530, K, Mythri P wrote:
> >
> >> > - What is dcofreq? Looking at the code, it tells if the pixel clock is >
> >> > 1000MHz. Why is such a field needed, can't the HDMI driver manage that
> >> > itself? And if it's needed, why is it called dcofreq, the name doesn't
> >> > make much sense to me.
> >> It is DCO frequency, It suggest the frequency selector range ,
> >
> > The field is not DCO frequency, it's a boolean, 0 or 1. That's why the
> > name doesn't really make sense to me.
> >
> >> HDMI_PLL_CONFIGURATION2 (3:1) has to be set accordingly by the driver
> >> depending on whether the CLKOUTLDO is greater than or less than
> >> 1000Mhz, but anyways the decision is taken by the driver.
> >
> > But can't it be done in the ti_hdmi driver, at the same time when
> > programming the registers? Why do we need to set the boolean beforehand.
> >
> It can be done, It is not a boolean , boolean logic is used to
> determine the value 0x2 / 0x4.
> Page # 101. (DCO frequency).

The code says:

        pi->dcofreq = phy > 1000 * 100;                                          

so it is a boolean.

> > What do you mean DSI PLL might not be sufficient? We can get higher
> > DISPC clocks with DSI PLL than with PRCM.
> Well , from the older calculation / values passed to DSI ,
> it was seen that DSI PLL was in the order of 156Mhz, and PRCM with 186Mhz.
> This had resulted in underflow issues with HDMI.

But that's just up to the particular configuration used. DSI PLL can be
configured to reach the maximum DSS FCLK, but with PRCM this is not
usually possible.

I see your point that it is relevant for HDMI to work properly, but
there are many other things that are also needed for HDMI to work
properly and we don't print them. And as the DISPC clk source is already
printed above in the DISPC section, I don't see the need for that here.

 Tomi