Re: [PATCH 2/2] OMAPDSS: Ensure OPP100 when DSS is operational

[Kevin Hilman <khilman@ti.com>]

[ Tomi, sorry for the delay.  I thought I had sent this a while back,
  but found it in my drafts folder. ]

+Mike for clock comments

Tomi Valkeinen <tomi.valkeinen@ti.com> writes:

> On Tue, 2012-03-13 at 11:37 -0700, Kevin Hilman wrote:
>> Hi Tomi,
>> 
>> Tomi Valkeinen <tomi.valkeinen@ti.com> writes:
>> 
>> > Hi Kevin, Paul,
>> >
>> > I know you're busy, but I'd appreciate a comment/ack on these two small
>> > patches, so I could get them in to next merge window. Otherwise using
>> > any other OPP than OPP100 will most likely break the DSS.
>> >
>> > This looks quite straightforward fix for me, but I'm not sure if there
>> > could be any side effects.
>> 
>> How does it affect OMAP3?  OPP50/OPP100 names are specific to OMAP4.
>
> They are? At least 3630 TRM speaks of them in the DSS chapter.
>

The OPP names might exist, but the freq/voltage values are different
between 3430, 3630 and 4430.

The point is that it's hard to make SoC independent code that depends on
a particular OPP because OPPs are defined as SoC specific.

>> Also, Can you help us understand the exact nature of the constraint?
>
> The TRM lists maximum clock rates for the DSS clocks. You should be able
> to find them by searching "OPP100" in the DSS chapter. In my TRMs there
> are:
>
> OMAP3630: Table 7-19. Display Subsystem Clocks
> OMAP4430: Table 10-4. DSS Clock Frequencies
>
>> It sounds to me like it acutally is a throughput constraint on CORE.  If
>> so, wouldn't it be clearer to set a throughput constraint that is
>> calculated based on the pixel clock and resulting bitrate that would
>> have the same effect?
>
> I don't see that these limits would have anything to do with CORE. I'm
> guessing that the DSS HW just can't function properly with high clocks
> and low voltage.

OK, this gets back to something we've talked about a few times that is
needed in the clock framework.  Basically, we need a way for clocks to
prevent changes so these kinds of dependencies can be tracked.  We've
talked about new APIs like clk_allow_changes(), clk_block_changes() or
something like that.

Basically, something that allows clocks to know that their will not be
changed under them. 

> Making a constraint for the throughput is another matter, which should
> be also fixed at some point. So in the future I hope we'll have PM
> constraints coming from two sources: 1) a calculation based on the
> memory throughput needs 

This can be done today.  That is the purpose of the tput constraint.

> 2) the minimum clock rates.

Right, today we don't have a way do to this, and clock framework support
will be needed.  I'll let Paul & Mike comment on that aspect, and
hopefully we'll have something in common clock that will be able to
handle this eventually.

> But both of those are non-trivial to code, so this patch aims to keep
> DSS working until those are implemented. Also, in practice, it's quite
> rare that the DSS clocks would all be below the limits in the tables.
> That could only happen with a fixed, known configuration with rather
> small displays.

So, in summary, I have no objection $SUBJECT patch which implements the
constraint using the only available method we have today.

Kevin