Re: [PATCH 3/7] drm/i915: move dp clock computations to encoder->compute_config

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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