On Thu, Oct 08, 2015 at 10:19:14AM +0200, Daniel Vetter wrote: > On Wed, Oct 07, 2015 at 10:08:25PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Apparently writing the DPLL register P1/P2 divider fields won't trigger > > an actual change in the DPLL output unless VGA mode is enabled for > > prior to the register write that changes the P1/P2 dividers. The write > > with the new P1/P2 divider can itself disable VGA mode again without > > problems. > > > > I tested the behaviour on my 946GZ, and when manually frobbing the > > register with the display on, the behaviour is very clear. However I > > can't explain why this machine actually works. The P1/P2 divider > > changes caused by normal modesets do seem to make it through to the > > hardware somehow since I get a stable picture on the monitor with > > any resolution. Maybe it's the "three times for luck" stuff that > > somehow masks the problem, or something. > > > > But apparently there are machines (eg. Nick Bowler's G45) where that > > isn't the case and we fail to get the correct clock from the DPLL. > > > > Things used to work because we enabled VGA mode for disabled DPLLs, > > so when re-enabling the DPLL VGA mode was enabled just prior to the > > first register write, and hence the P1/P2 change went through without > > a hitch. That got changed in > > > > b8afb9113c51 drm/i915: Keep GMCH DPLL VGA mode always disabled > > > > in the name of consistency. In order to keep the consistency part, > > leave VGA mode disabled for disabled DPLLs, but turn it on just prior > > to updating the P1/P2 dividers to make sure the hardware picks up > > on the new values. > > > > Cc: Nick Bowler <nbowler@xxxxxxxxxx> > > Reported-by: Nick Bowler <nbowler@xxxxxxxxxx> > > Tested-by: Nick Bowler <nbowler@xxxxxxxxxx> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_display.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index f4fdff9..ce51dbc 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -1743,6 +1743,13 @@ static void i9xx_enable_pll(struct intel_crtc *crtc) > > I915_READ(DPLL(!crtc->pipe)) | DPLL_DVO_2X_MODE); > > } > > > > + /* > > + * Apparently we need to have VGA mode enabled prior to changing > > + * the P1/P2 dividers. Otherwise the DPLL will keep using the old > > + * dividers, even though the register value does change. > > + */ > > + I915_WRITE(reg, 0); > > Again POSTING_READ for ordering? Otherwise Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > Not that there's a lot to review since Bspec is silent ... One option is to start using mmiowb() for documenting the write ordering (which is a no-op on x86 and so heaven forbid we get to use WC ever.) -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
