On Thu, 23 May 2013 17:30:06 +0200 Daniel Vetter <daniel at ffwll.ch> wrote: > On Tue, May 14, 2013 at 05:08:27PM -0700, Jesse Barnes wrote: > > We need this for comparing modes between configuration changes. > > > > v2: try harder to calulate non-simple pixel clocks (Daniel) > > call get_clock after getting the encoder config, needed for pixel multiply > > (Jesse) > > v3: drop get_clock now that the pixel_multiply has been moved into > > get_pipe_config > > v4: re-add get_clock; we need to get the pixel multiplier in the > > encoder, so need to calculate the clock value after the encoder's > > get_config is called > > v5: drop hsw clock_get, still needs to be written > > > > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org> > > Two things: > - the pixel_multiplier should be a separate patch, and I think we should > add pipe_config check support for it. Maybe if we pick a default value > of 1 (both for the get_config hw state readout and the compute config > stuff in the modeset code) that should also simplify a bit since only > SDVO uses it atm. We still need to implement pixel multiplier stuff for > native hdmi ports ... Yeah, I'm ambivalent about it. I like it called out in the encoder funcs just because the setting belongs there, but since only SDVO uses it, it does seem wasteful. Maybe I'm just being hopeful about the HDMI side of things landing soon. :) > - For the clock readout code I think we should be able to have pipe config > compare support (with adjusted_mode->clock), with a bit of fuzz at > least. Not on current dinq, but with my cleanup to give > adjusted_mode->clock saner semantics: > > http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/21750 So far I haven't needed the fuzz, but I'm not opposed to adding it as needed. > - There's no reason imo for the new ->get_clock vfunc, it splits exactly > the same as the existing ->get_pipe_config. So could simply be included > there. Except that it depends on the results of the encoder->get_config callback... that's the only reason I split it. If you have ideas about how to merge it back that's ok with me. > With those three adjustements I think this is ready to go. For the > follow-up patches though I think we should guard them with a fastboot > module option. Yeah, the main one there is the compute_config. We can hide that behind "boot_me_faster" or some such. -- Jesse Barnes, Intel Open Source Technology Center
