On Fri, 2015-05-15 at 13:56 +0300, Jani Nikula wrote: > On Fri, 15 May 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx> wrote: > > In the following commit, the place where the contents of dpll_hw_state > > in crtc_state where zeroed was changed. Prior to that commit, it > > happened when the new state was allocated, but now that happens just > > before the call the .crtc_compute_clock() hook. The DP code for SKL, > > however, sets up the (private) PLL in the encoder compute config > > function that has already run by the time that memset() is reached, > > causing the previous value to be lost. > > > > This patch fixes the issue by moving the memset() down the call chain, > > so that it is only called if the values in dpll_hw_state are going to be > > updated. > > > > commit 4978cc93d9ac240b435ce60431aef24239b4c270 > > Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx> > > Date: Tue Apr 21 17:13:21 2015 +0300 > > > > drm/i915: Preserve shared DPLL information in new pipe_config > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90462 > > --- > > > > On Fri, 2015-05-15 at 13:31 +0300, Ander Conselvan De Oliveira wrote: > >> 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. > > > > Like this completely untested patch. > > Missing sob. Oops. Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx> > > Jani. > > > > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 9 +++++++++ > > drivers/gpu/drm/i915/intel_display.c | 8 ++++++-- > > drivers/gpu/drm/i915/intel_dp.c | 3 +++ > > 3 files changed, 18 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index ec5d2ea..48d45b2 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1087,6 +1087,9 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc, > > WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) | > > WRPLL_DIVIDER_POST(p); > > > > + memset(&crtc_state->dpll_hw_state, 0, > > + sizeof(crtc_state->dpll_hw_state)); > > + > > crtc_state->dpll_hw_state.wrpll = val; > > > > pll = intel_get_shared_dpll(intel_crtc, crtc_state); > > @@ -1309,6 +1312,9 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc, > > } else /* eDP */ > > return true; > > > > + memset(&crtc_state->dpll_hw_state, 0, > > + sizeof(crtc_state->dpll_hw_state)); > > + > > crtc_state->dpll_hw_state.ctrl1 = ctrl1; > > crtc_state->dpll_hw_state.cfgcr1 = cfgcr1; > > crtc_state->dpll_hw_state.cfgcr2 = cfgcr2; > > @@ -1419,6 +1425,9 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > > } > > } > > > > + memset(&crtc_state->dpll_hw_state, 0, > > + sizeof(crtc_state->dpll_hw_state)); > > + > > crtc_state->dpll_hw_state.ebb0 = > > PORT_PLL_P1(clk_div.p1) | PORT_PLL_P2(clk_div.p2); > > crtc_state->dpll_hw_state.pll0 = clk_div.m2_int; > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 8d40d7d..a7732b4 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -7475,6 +7475,9 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc, > > struct drm_connector_state *connector_state; > > int i; > > > > + memset(&crtc_state->dpll_hw_state, 0, > > + sizeof(crtc_state->dpll_hw_state)); > > + > > for_each_connector_in_state(state, connector, connector_state, i) { > > if (connector_state->crtc != &crtc->base) > > continue; > > @@ -8516,6 +8519,9 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc, > > bool is_lvds = false; > > struct intel_shared_dpll *pll; > > > > + memset(&crtc_state->dpll_hw_state, 0, > > + sizeof(crtc_state->dpll_hw_state)); > > + > > is_lvds = intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS); > > > > WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)), > > @@ -12265,8 +12271,6 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state) > > if (needs_modeset(crtc_state)) { > > clear_pipes |= 1 << intel_crtc->pipe; > > intel_crtc_state->shared_dpll = DPLL_ID_PRIVATE; > > - memset(&intel_crtc_state->dpll_hw_state, 0, > > - sizeof(intel_crtc_state->dpll_hw_state)); > > } > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 5bd73ad..6fd9c60 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -1097,6 +1097,9 @@ skl_edp_set_pll_config(struct intel_crtc_state *pipe_config, int link_clock) > > { > > u32 ctrl1; > > > > + memset(&pipe_config->dpll_hw_state, 0, > > + sizeof(pipe_config->dpll_hw_state)); > > + > > pipe_config->ddi_pll_sel = SKL_DPLL0; > > pipe_config->dpll_hw_state.cfgcr1 = 0; > > pipe_config->dpll_hw_state.cfgcr2 = 0; > > -- > > 2.1.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx