On Wed, 2015-05-13 at 17:56 +0100, Damien Lespiau wrote: > On Wed, May 13, 2015 at 05:40:44PM +0100, Tvrtko Ursulin wrote: > > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > > > Since commit 4978cc93d9ac240b435ce60431aef24239b4c270 started clearing > > dpll state and recomputing it via crtc_compute_clock (and probably some > > other commit which triggered pipe config checking), modesetting is now > > constantly triggering warnings about dpll_hw_state.ctrl1 mismatch. > > > > Reason is crtc_compute_clock calls skl_ddi_pll_select which does not do > > anything for eDP, leaving the ctrl1 state at the default of zero. > > > > This potentially hacky fix makes skl_ddi_pll_select call > > skl_edp_set_pll_config which fixes the problem for me. > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > Cc: Damien Lespiau <damien.lespiau@xxxxxxxxx> > > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx> > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Nop! (at least I really don't think so). > > As discussed on IRC, on DDI platforms, the (e)DP compute_config() does > the private DPLL selection, and the ddi_pll_select() does the the same > for shared DPLLs (I'm not saying that the end result we want, just how > it works today). That split comes from the introduction of shared_dpll > in the DDI PLL selection last summer. > > Anyway, this means that, for SKL, dpll_hw_state is touched by the > encoder's compute_config() for (e)DP. > > I think we could just remove the memset() in 4978cc93 (maybe?), or try > to unify a bit better and only have one place where we do PLL selection > (which I assume is part of the bigger atomic plan). Not > skl_edp_set_pll_config() in both compute_config() and ddi_pll_select() > though. The memset() was added to fix a similar warning on a different platform, but I can't remember which one now. Perhaps a more immediate fix would be to move the memset() down the call chain. Long term I think we should remove all the assumptions of initial values from the compute config path. Having only one place that selects PLL would also be nice IMO. Ander _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
