>>-----Original Message----- >>From: Menon, Nishanth >>Sent: Thursday, August 26, 2010 6:00 AM >>To: Kevin Hilman >>Cc: Gopinath, Thara; linux-omap@xxxxxxxxxxxxxxx >>Subject: RE: [PM-OPP][PATCH] OMAP: Modifying the frequency comparison logic. >> >>> -----Original Message----- >>> From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >>> Sent: Wednesday, August 25, 2010 5:41 PM >>> To: Menon, Nishanth >>> Cc: Gopinath, Thara; linux-omap@xxxxxxxxxxxxxxx >>> Subject: Re: [PM-OPP][PATCH] OMAP: Modifying the frequency comparison >>> logic. >>> >>> "Menon, Nishanth" <nm@xxxxxx> writes: >>> >>> >> From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >>> >> Sent: Tuesday, August 24, 2010 4:14 PM >>> > >>> > >>> >> >>> >> "Menon, Nishanth" <nm@xxxxxx> writes: >>> >> >>> >> >> >>> >> >> thara gopinath <thara@xxxxxx> writes: >>> >> >> >>> >> >> > From: Thara Gopinath <thara@xxxxxx> >>> >> >> > >>> > >>> > [...] >>> >> >> > >>> >> >> > The above is not correct as we expect the framework to return back >>> >> >> > the opp table entry corresponding to 266 Mhz. >>> >> >> > >>> > >>> > [...] >>> >> >> > >>> >> >> > b. Do the comparison in Mhz in the opp layer rather than in Hz. >>> >> >> > This would mean we will divide the rate passed into the opp >>> layer >>> >> >> > API and the rates stored in the opp tables by 1000000 to get >>> the >>> >> >> > rates in Mhz and do the necessary comparision. In this >>> approach >>> >> >> any >>> >> >> > vague frequency like 266.045Mhz will get mapped to 266 Mhz >>> in the >>> >> >> > opp table. But if the passed rate is 267 Mhz, the opp >>> framework >>> >> >> > will still rerturn an error or the next highest opp table >>> entry >>> >> >> > >>> >> >> > This patch implements solution b. The scenario mentioned above is >>> >> >> > esp true for OMAP4 dpll_iva where we do end up with such weird >>> >> >> frequencies >>> >> >> > due to sys clk being at 38.4 Mhz. >>> >> >> >>> >> >> I agree that solution b is better, although it makes the '_exact' >>> >> >> function a bit less exact. :/ >>> >> >> >>> >> >> solution b is fine with me, but the kerneldoc for these find >>> functions >>> >> >> should be updated to describe the new matching technique. >>> >> > >>> >> > I agree, I suggest one improvement though - the search accuracy will >>> >> vary >>> >> > Based on the silicon rev, one size will probably not fit every >>> silicon >>> >> and >>> >> > Domains we have - I suggest having accuracy as a parameter as part of >>> >> domain >>> >> > Registration/configurable parameter >>> >> > e.g. >>> >> >> > + unsigned long rate = temp_opp->rate / 1000000; >>> >> > Will probably configurable to the "exactness" we expect to handle per >>> >> domain/silicon family. >>> >> > >>> >> >>> >> The more I think about, I think we should leave the 'exact' find the >>> way >>> >> it is, especially as we move to device OPPs we will probably want to >>> >> have more precise matching. >>> >> >>> >> What about adding another function that does a "find closest"? >>> >>> > Just my 2cents: With accuracy as a param? >>> >>> Why would you need accuracy for "find closest"? >>> >>How close should "closest" be? There has to be a parameter for this - >>something like as close as 10Mhz, but beyond the match fails. >>Currently this patch introduces a hardcoded accuracy which could be >>Carried forward. >> >>But, On different silicon the deltas due to sysclk can vary - there is no >>silver bullet algo we can write which will be accurate for all silicon and >>domain combination.. >> >> >>Further, is'nt ceil and floor functions we have a more specific >>Implementation of "closest"? in the sense, it says closest to which >>Direction - up or down.. >> >>[...] >>Rest of the discussion seems aligned. Hello Kevin, I am not sure about the conclusion of this discussion. For me we should avoid putting possible weird frequencies like 266789501.. Hence the round off to /10^6. If on some platforms due to sys clk we have frequency 267 instead of 266, I would prefer to edit the opp table through the opp framework APIs. This is my opinion. I may not have fully understood the concerns discussed in this forum. Regards Thara -- 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