On Mon, 18 Dec 2017, "Chauhan, Madhav" <madhav.chauhan@xxxxxxxxx> wrote: >> -----Original Message----- >> From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of >> Jani Nikula >> Sent: Thursday, October 12, 2017 9:36 PM >> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> Cc: Nikula, Jani <jani.nikula@xxxxxxxxx>; Daniel Vetter >> <daniel.vetter@xxxxxxxx> >> Subject: [PATCH 4/4] drm/i915: push shared dpll enable to >> encoders on DDI platforms >> >> As we move more encoder specific stuff to encoders on DDI platforms, >> follow suit with shared dpll enable. In the future, we'll need this refactoring >> for further encoder specific details in the middle of the enable sequence. >> >> v2: drop ifs around the intel_enable_shared_dpll calls (Daniel) >> >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/intel_crt.c | 2 ++ >> drivers/gpu/drm/i915/intel_ddi.c | 16 ++++++++++++++++ >> drivers/gpu/drm/i915/intel_display.c | 3 --- >> 3 files changed, 18 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_crt.c >> b/drivers/gpu/drm/i915/intel_crt.c >> index 67f771f24608..65f28062cb51 100644 >> --- a/drivers/gpu/drm/i915/intel_crt.c >> +++ b/drivers/gpu/drm/i915/intel_crt.c >> @@ -255,6 +255,8 @@ static void hsw_pre_pll_enable_crt(struct >> intel_encoder *encoder, >> WARN_ON(!intel_crtc->config->has_pch_encoder); >> >> intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false); >> + >> + intel_enable_shared_dpll(intel_crtc); >> } >> >> static void hsw_pre_enable_crt(struct intel_encoder *encoder, diff --git >> a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c >> index 9819e51fa160..05f9db9c3d52 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -2416,13 +2416,27 @@ static void intel_disable_ddi(struct intel_encoder >> *intel_encoder, >> } >> } >> >> +static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder, >> + const struct intel_crtc_state *pipe_config, >> + const struct drm_connector_state >> *conn_state) { >> + struct drm_crtc *crtc = pipe_config->base.crtc; >> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> + >> + intel_enable_shared_dpll(intel_crtc); >> +} >> + > > Shouldn’t we add this code (and below as well) inside enable() or pre_enable() functions > of struct intel_encoder as pre_pll_enable() function should have some code prior to enabling PLL. The first step should be pushing the calls down to encoder hooks while keeping the modeset sequence as close as possible to before. After that, we can start tweaking the order where applicable. Always make minimal changes so you see where stuff breaks. BR Jani. > > Regards, > Madhav > >> static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder, >> const struct intel_crtc_state *pipe_config, >> const struct drm_connector_state >> *conn_state) { >> + struct drm_crtc *crtc = pipe_config->base.crtc; >> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> uint8_t mask = pipe_config->lane_lat_optim_mask; >> >> bxt_ddi_phy_set_lane_optim_mask(encoder, mask); >> + >> + intel_enable_shared_dpll(intel_crtc); >> } >> >> void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp) @@ -2730,6 >> +2744,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum >> port port) >> intel_encoder->enable = intel_enable_ddi; >> if (IS_GEN9_LP(dev_priv)) >> intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable; >> + else >> + intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable; >> intel_encoder->pre_enable = intel_ddi_pre_enable; >> intel_encoder->disable = intel_disable_ddi; >> intel_encoder->post_disable = intel_ddi_post_disable; diff --git >> a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index 6ed299670f27..d9ccde0a8097 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -5481,9 +5481,6 @@ static void haswell_crtc_enable(struct >> intel_crtc_state *pipe_config, >> >> intel_encoders_pre_pll_enable(crtc, pipe_config, old_state); >> >> - if (intel_crtc->config->shared_dpll) >> - intel_enable_shared_dpll(intel_crtc); >> - >> if (intel_crtc_has_dp_encoder(intel_crtc->config)) >> intel_dp_set_m_n(intel_crtc, M1_N1); >> >> -- >> 2.11.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
