On Mon, Dec 15, 2014 at 09:21:41PM +0200, Ville Syrjälä wrote:
> On Mon, Dec 15, 2014 at 10:37:57AM -0800, Matt Roper wrote:
> > On Thu, Dec 11, 2014 at 02:38:07PM +0200, Ander Conselvan de Oliveira wrote:
> > > @@ -7541,7 +7541,7 @@ static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
> > > void intel_dp_get_m_n(struct intel_crtc *crtc,
> > > struct intel_crtc_state *pipe_config)
> > > {
> > > - if (crtc->config.has_pch_encoder)
> > > + if (pipe_config->has_pch_encoder)
> >
> > I think this one might also change the semantics? The callchain
> >
> > check_crtc_state() -> intel_{dp,ddi}_get_config() -> intel_dp_get_m_n()
> >
> > passes down a fresh pipe_config() that isn't the same structure stored
> > in crtc->config (although it hopefully has the same values in the end).
>
> This looks like a straight up bug fix to me. Apparently I already
> fumbled it when I introduced the DP M/N readout.
>
> The state checker hasn't tripped on this since we've already
> written the new pipe config to crtc->config by the time we do the
> readout during modeset.
>
> And intel_modeset_readout_hw_state() reads the state directly into
> crtc->config, so even the initial state would have come out correct.
>
> So, while the code is wrong, doesn't look like it could have caused
> any real issues.
We also have very light coverage of direct modeset changes (i.e. without
going through off in-between). Iirc most of the igts shut down stuff in
between before setting the new modes (because that's the only way to get
somewhat reliably modesets without atomic updates). I think we should
expect more fun in this area once we have real atomic modesets.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
