| Message ID | 20180502002637.13928-1-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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))
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. 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(-)