On Wed, May 02, 2018 at 02:58:51PM -0700, Paulo Zanoni wrote: > For all platforms that run haswell_crtc_enable, our spec tells us to > configure the transcoder clocks and do link training before it tells > us to set pipeconf and the other pipe/transcoder/plane registers.
oh! I remember this order years ago when trying to figure out where to enable PSR... but I never noticed we weren't fully respecting it. > > Starting from Icelake, we get machine hangs if we try to touch the > pipe/transcoder registers without having the clocks configured and not > having some chicken bits set. So this patch changes > haswell_crtc_enable() to issue the calls at the appropriate order > mandated by the spec. > > While setting the appropriate chicken bits would also work here, it's > better if we actually program the hardware the way it is intended to > be programmed. And the chicken bit also has some theoretical downsides > that may or may not affect us. Also, correctly programming the > hardware does not prevent us from setting the chicken bits in a later > patch in case we decide to. > > v2: Don't forget link training (Ville). > > Cc: Arthur J Runyan <arthur.j.run...@intel.com> > Cc: James Ausmus <james.aus...@intel.com> > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zan...@intel.com> makes sense for me.... let's now just see CI is happy as well! ;) Reviewed-by: Rodrigo Vivi <rodrigo.v...@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > Again, I didn't test this patch on every affected platform. Let's see > what the CI system says about it. > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 1087358f6364..f566c9e56cf6 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5559,6 +5559,11 @@ static void haswell_crtc_enable(struct > intel_crtc_state *pipe_config, > if (intel_crtc->config->shared_dpll) > intel_enable_shared_dpll(intel_crtc); > > + intel_encoders_pre_enable(crtc, pipe_config, old_state); > + > + if (!transcoder_is_dsi(cpu_transcoder)) > + intel_ddi_enable_pipe_clock(pipe_config); > + > if (intel_crtc_has_dp_encoder(intel_crtc->config)) > intel_dp_set_m_n(intel_crtc, M1_N1); > > @@ -5587,11 +5592,6 @@ static void haswell_crtc_enable(struct > intel_crtc_state *pipe_config, > > intel_crtc->active = true; > > - intel_encoders_pre_enable(crtc, pipe_config, old_state); > - > - if (!transcoder_is_dsi(cpu_transcoder)) > - intel_ddi_enable_pipe_clock(pipe_config); > - > /* Display WA #1180: WaDisableScalarClockGating: glk, cnl */ > psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) && > intel_crtc->config->pch_pfit.enabled; > -- > 2.14.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
