Re: [PATCH 2/2] drm/i915: Enable DPLL VGA mode before P1/P2 divider write

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Oct 07, 2015 at 10:08:25PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 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@draconx.ca>
> Reported-by: Nick Bowler <nbowler@draconx.ca>
> Tested-by: Nick Bowler <nbowler@draconx.ca>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  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@ffwll.ch>
Not that there's a lot to review since Bspec is silent ...
-Daniel
> +
>  	I915_WRITE(reg, dpll);
>  
>  	/* Wait for the clocks to stabilize. */
> -- 
> 2.4.9
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx