Re: [PATCH 04/11] drm/i915: Factor out i9xx_compute_clocks() like ironlake_compute_clocks()

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Oct 31, 2012 at 07:04:29PM +0200, Ville Syrjälä wrote:
> On Wed, Oct 31, 2012 at 05:28:40PM +0100, Daniel Vetter wrote:
> > On Wed, Oct 31, 2012 at 05:50:17PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Split the i9xx clock stuff out from i9xx_compute_clocks().
> > > 
> > > Only compile tested!
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I'm working on a massive overhaul of the clock computation madness, so
> > that we do all that stuff before we start to touch the hw (and so would
> > finally have a reasonable chance at getting global bw issues right).
> 
> I was sure that the compute_clock() funcs already satisfied that.
> Perhaps I didn't look hard enough.
> 
> The reason for this patch actually was that I'm already using that
> approach in the atomic modeset code. Ie. I call compute_clock() for
> all modified CRTCs before touching the hardware.
> 
> Or is the problem simply that multiple calls to compute_clock() would
> depend on some bit of state that is changed somewhere later in
> crtc_mode_set()? So that when doing a multi-CRTC modeset, the final
> state is never seen by any of the compute_clock() funcs when used this
> way?

Well the plan is to compute all this state into struct intel_crtc_config
and then switch the code that changes the hw state to only use that
precomputed configuration instead of recomputing it. The reason for that
intermediate state keeping is twofold:
- We need to have some much better notion of a pipe configuration for
  fastboot anyway (including things like pll sharing and exact dotclocks).
  Using the same data structure for both the hw readout code needed for
  fastboot and in our own modeset code should allow for some nice
  consistency checks.
- We have a bunch of rippling dependencies in our state computation, e.g.
  depending upon global configuration we have different amounts of
  bandwidth available, which (should) affect the pipe bpp and so the
  clocks we select, in turn having effects on how we can share plls. If we
  try to run these computations twice, once in the ->check callback and
  once when changing the hw state we'll never get a perfect match. Hence I
  want to precompute and store all values into that pipe_config.

Essentially that branch is trying to implement all the atomic modeset
stuff without actually having an atomic modeset ioctl ;-) My aim is that
in the end I can detect fdi b/c link sharing constraints, dither the other
pipe to a lower pipe_bpp, add it to the array of pipe that need a full
modeset and then enable everything. Also, while I'm at it a want to kill
all these messy layering inversions where the crtc code pokes around in
various encoders ... It's not quite there yet ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch