Re: [PATCH] drm/i915: Stash desired logical vco in a reliable place.

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

On Tue, May 01, 2018 at 05:26:37PM -0700, Rodrigo Vivi wrote:
> On intel_dp_compute_config() we calculate the needed vco
> for eDP on gen9 and we stash it in
> intel_atomic_state.cdclk.logical.vco
> 
> However few moments later on intel_modeset_checks() we fully
> replace entire intel_atomic_state.cdclk.logical with
> dev_priv->cdclk.logical fully overwriting the logical desired
> vco.

Hmm. Yeah, that's a problem. However we can't just overwrite
the current state like you propose in the middle of the atomic
check phase.

I guess what we could do is move the code from intel_dp_compute_config()
into skl_modeset_calc_cdclk(). That way we wouldn't have to add new
encoder hooks or anything like that. Something like this perhaps?

static int skl_dpll0_vco(struct intel_atomic_state *state)
{
	vco = state->cdclk.logical.vco;
	if (!vco)
		vco = dev_priv->skl_preferred_vco_freq;

	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
		if (!crtc_state->base.enable)
			continue;

		if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
			continue;
	
		<insert code from intel_dp_compute_config()>
	}

	return vco;
}

skl_modeset_calc_cdclk()
{
	...
-	vco = intel_state->cdclk.logical.vco;
-	if (!vco)
-		vco = dev_priv->skl_preferred_vco_freq;
+	vco = skl_dpll0_vco(intel_state);
	...
}

> 
> So, with wrong VCO value we end up with wrong desired cdclk, but
> also it will raise a lot of WARNs: On gen9, when we read
> CDCLK_CTL to verify if we configured properly the desired
> frequency the CD Frequency Select bits [27:26] == 10b can mean
> 337.5 or 308.57 MHz depending on the VCO. So if we have wrong
> VCO value stashed we will believe the frequency selection didn't
> stick and start to raise WARNs of cdclk mismatch.
> 
> [   42.857519] [drm:intel_dump_cdclk_state [i915]] Changing CDCLK to 308571 kHz, VCO 8640000 kHz, ref 24000 kHz, bypass 24000 kHz, voltage level 0
> [   42.897269] cdclk state doesn't match!
> [   42.901052] WARNING: CPU: 5 PID: 1116 at drivers/gpu/drm/i915/intel_cdclk.c:2084 intel_set_cdclk+0x5d/0x110 [i915]
> [   42.938004] RIP: 0010:intel_set_cdclk+0x5d/0x110 [i915]
> [   43.155253] WARNING: CPU: 5 PID: 1116 at drivers/gpu/drm/i915/intel_cdclk.c:2084 intel_set_cdclk+0x5d/0x110 [i915]
> [   43.170277] [drm:intel_dump_cdclk_state [i915]] [hw state] 337500 kHz, VCO 8100000 kHz, ref 24000 kHz, bypass 24000 kHz, voltage level 0
> [   43.182566] [drm:intel_dump_cdclk_state [i915]] [sw state] 308571 kHz, VCO 8640000 kHz, ref 24000 kHz, bypass 24000 kHz, voltage level 0
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 83da50b13d81..be066afaefa7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1946,7 +1946,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  			break;
>  		}
>  
> -		to_intel_atomic_state(pipe_config->base.state)->cdclk.logical.vco = vco;
> +		dev_priv->cdclk.logical.vco = vco;
>  	}
>  
>  	if (!HAS_DDI(dev_priv))
> -- 
> 2.13.6

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx