On Thu, Apr 25, 2013 at 02:04:45PM +0200, Daniel Vetter wrote: > On Thu, Apr 25, 2013 at 02:34:39PM +0300, Ville Syrj?l? wrote: > > On Fri, Apr 19, 2013 at 11:14:33AM +0200, Daniel Vetter wrote: > > > With the exception of hsw, which has dedicated DP clocks which run at > > > the fixed frequency already, and vlv, which doesn't have optmized > > > pre-defined dp clock parameters (yet). > > > > So it looks like were still going through the full compute clocks thing, > > which will possibly produce something different, and then we overwrite it > > with the fixed clocks afterwards. I'm assuming that's just a transitional > > issue and will get fixed later. > > Yeah, I guess I should have spilled a few more words about where I > eventually want to end up with this. > > So ultimately my idea is that in the compute config stage first the crtc > code puts the default platform pll limits into the pipe_config. Then > encoders can either overwrite that limit structure with their own special > stuff (mostly for lvds madness). Or they can pick some or all of the > parameters (e.g. just the p2 switchover on hdmi, or all the clock > parameters for dp/sdvo tv). > > Once that's done then the generic crtc code can fill out any missing bits > (using the find_best_pll code) and then try to assign which pll to use (if > it's a platform with shared plls). In the end the modeset could should > simply write the computed stuff into registers and never be able to fail. > > Of course there's still a lot of data to be moved into pipe_config to make > this all happen, hence some of the temporary ugliness. > > > The only real concern I had was that the fixed clocks don't include the > > derived m factor for example, but it looks like we shouldn't need that > > except for the LVDS reduced clock thing. > > The idea behind leaving out m (and compute dotclock/vco) from the > pipe_config stuff is that we don't store that into the hw. It's only > really used in the pll computation. So once this has all settled I think > we should rip this all out from the clock structure and compute it > as-needed in the find_best_pll functions. I'm working on some patches > already to move into that direction a bit. Yeah I had the same idea about always computing the derived stuff on-demand always. Would remove the risk of accidentally using an unpopulated value. Of course it wasn't such a big issue before my intel_clock_to == struct dpll patch, since you didn't have the derived stuff in the struct dpll in the first place. Win some, lose some. -- Ville Syrj?l? Intel OTC