Re: [Intel-gfx] [PATCH 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL
On Fri, Feb 10, 2017 at 03:29:59PM +0200, Ander Conselvan de Oliveira wrote: > According to bspec, the DDI IO power domains should be enabled after > enabling the DPLL and mapping it to the DDI. The current order doesn't > seem to create problems with Skylake and Kabylake, but causes enable > timeouts in Geminilake. > > Cc: David Weinehall > Signed-off-by: Ander Conselvan de Oliveira > > --- > drivers/gpu/drm/i915/i915_drv.h | 5 +++ > drivers/gpu/drm/i915/intel_ddi.c| 49 > drivers/gpu/drm/i915/intel_display.c| 12 ++ > drivers/gpu/drm/i915/intel_drv.h| 3 ++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 68 > +++-- > 5 files changed, 108 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index bfccf9d..27847d4 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -343,6 +343,11 @@ enum intel_display_power_domain { > POWER_DOMAIN_PORT_DDI_C_LANES, > POWER_DOMAIN_PORT_DDI_D_LANES, > POWER_DOMAIN_PORT_DDI_E_LANES, > + POWER_DOMAIN_PORT_DDI_A_IO, > + POWER_DOMAIN_PORT_DDI_B_IO, > + POWER_DOMAIN_PORT_DDI_C_IO, > + POWER_DOMAIN_PORT_DDI_D_IO, > + POWER_DOMAIN_PORT_DDI_E_IO, > POWER_DOMAIN_PORT_DSI, > POWER_DOMAIN_PORT_CRT, > POWER_DOMAIN_PORT_OTHER, > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index b0c4d23..72754b9 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1440,6 +1440,18 @@ bool intel_ddi_get_hw_state(struct intel_encoder > *encoder, > return ret; > } > > +static unsigned long long > +intel_ddi_get_power_domains(struct intel_encoder *encoder) > +{ > + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); > + enum pipe pipe; > + > + if (intel_ddi_get_hw_state(encoder, &pipe)) > + return BIT_ULL(dig_port->ddi_io_power_domain); > + > + return 0; > +} > + > void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc) > { > struct drm_crtc *crtc = &intel_crtc->base; > @@ -1682,6 +1694,7 @@ static void intel_ddi_pre_enable_dp(struct > intel_encoder *encoder, > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > enum port port = intel_ddi_get_encoder_port(encoder); > + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); > > intel_dp_set_link_params(intel_dp, link_rate, lane_count, >link_mst); > @@ -1689,6 +1702,9 @@ static void intel_ddi_pre_enable_dp(struct > intel_encoder *encoder, > intel_edp_panel_on(intel_dp); > > intel_ddi_clk_select(encoder, pll); > + > + intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain); > + > intel_prepare_dp_ddi_buffers(encoder); > intel_ddi_init_dp_buf_reg(encoder); > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > @@ -1708,9 +1724,13 @@ static void intel_ddi_pre_enable_hdmi(struct > intel_encoder *encoder, > struct drm_encoder *drm_encoder = &encoder->base; > enum port port = intel_ddi_get_encoder_port(encoder); > int level = intel_ddi_hdmi_level(dev_priv, port); > + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); > > intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); > intel_ddi_clk_select(encoder, pll); > + > + intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain); > + > intel_prepare_hdmi_ddi_buffers(encoder); > if (IS_GEN9_BC(dev_priv)) > skl_ddi_set_iboost(encoder, level); > @@ -1754,6 +1774,7 @@ static void intel_ddi_post_disable(struct intel_encoder > *intel_encoder, > struct drm_encoder *encoder = &intel_encoder->base; > struct drm_i915_private *dev_priv = to_i915(encoder->dev); > enum port port = intel_ddi_get_encoder_port(intel_encoder); > + struct intel_digital_port *dig_port = enc_to_dig_port(encoder); > int type = intel_encoder->type; > uint32_t val; > bool wait = false; > @@ -1793,6 +1814,8 @@ static void intel_ddi_post_disable(struct intel_encoder > *intel_encoder, > > intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); > } > + > + intel_display_power_put(dev_priv, dig_port->ddi_io_power_domain); > } > > void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder, > @@ -2190,12 +2213,38 @@ void intel_ddi_init(struct drm_i915_private > *dev_priv, enum port port) > intel_encoder->get_hw_state = intel_ddi_get_hw_state; > intel_encoder->get_config = intel_ddi_get_config; > intel_encoder->suspend = intel_dp_encoder_suspend; > + intel_encoder->get_power_domains = intel_ddi_get_power_domains; > > intel_dig_port->port = port; > intel_dig_
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL
On Fri, Feb 10, 2017 at 03:29:59PM +0200, Ander Conselvan de Oliveira wrote: > According to bspec, the DDI IO power domains should be enabled after > enabling the DPLL and mapping it to the DDI. The current order doesn't > seem to create problems with Skylake and Kabylake, but causes enable > timeouts in Geminilake. > > Cc: David Weinehall > Signed-off-by: Ander Conselvan de Oliveira > Reviewed-by: David Weinehall > --- > drivers/gpu/drm/i915/i915_drv.h | 5 +++ > drivers/gpu/drm/i915/intel_ddi.c| 49 > drivers/gpu/drm/i915/intel_display.c| 12 ++ > drivers/gpu/drm/i915/intel_drv.h| 3 ++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 68 > +++-- > 5 files changed, 108 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index bfccf9d..27847d4 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -343,6 +343,11 @@ enum intel_display_power_domain { > POWER_DOMAIN_PORT_DDI_C_LANES, > POWER_DOMAIN_PORT_DDI_D_LANES, > POWER_DOMAIN_PORT_DDI_E_LANES, > + POWER_DOMAIN_PORT_DDI_A_IO, > + POWER_DOMAIN_PORT_DDI_B_IO, > + POWER_DOMAIN_PORT_DDI_C_IO, > + POWER_DOMAIN_PORT_DDI_D_IO, > + POWER_DOMAIN_PORT_DDI_E_IO, > POWER_DOMAIN_PORT_DSI, > POWER_DOMAIN_PORT_CRT, > POWER_DOMAIN_PORT_OTHER, > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index b0c4d23..72754b9 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1440,6 +1440,18 @@ bool intel_ddi_get_hw_state(struct intel_encoder > *encoder, > return ret; > } > > +static unsigned long long > +intel_ddi_get_power_domains(struct intel_encoder *encoder) > +{ > + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); > + enum pipe pipe; > + > + if (intel_ddi_get_hw_state(encoder, &pipe)) > + return BIT_ULL(dig_port->ddi_io_power_domain); > + > + return 0; > +} > + > void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc) > { > struct drm_crtc *crtc = &intel_crtc->base; > @@ -1682,6 +1694,7 @@ static void intel_ddi_pre_enable_dp(struct > intel_encoder *encoder, > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > enum port port = intel_ddi_get_encoder_port(encoder); > + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); > > intel_dp_set_link_params(intel_dp, link_rate, lane_count, >link_mst); > @@ -1689,6 +1702,9 @@ static void intel_ddi_pre_enable_dp(struct > intel_encoder *encoder, > intel_edp_panel_on(intel_dp); > > intel_ddi_clk_select(encoder, pll); > + > + intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain); > + > intel_prepare_dp_ddi_buffers(encoder); > intel_ddi_init_dp_buf_reg(encoder); > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > @@ -1708,9 +1724,13 @@ static void intel_ddi_pre_enable_hdmi(struct > intel_encoder *encoder, > struct drm_encoder *drm_encoder = &encoder->base; > enum port port = intel_ddi_get_encoder_port(encoder); > int level = intel_ddi_hdmi_level(dev_priv, port); > + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); > > intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); > intel_ddi_clk_select(encoder, pll); > + > + intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain); > + > intel_prepare_hdmi_ddi_buffers(encoder); > if (IS_GEN9_BC(dev_priv)) > skl_ddi_set_iboost(encoder, level); > @@ -1754,6 +1774,7 @@ static void intel_ddi_post_disable(struct intel_encoder > *intel_encoder, > struct drm_encoder *encoder = &intel_encoder->base; > struct drm_i915_private *dev_priv = to_i915(encoder->dev); > enum port port = intel_ddi_get_encoder_port(intel_encoder); > + struct intel_digital_port *dig_port = enc_to_dig_port(encoder); > int type = intel_encoder->type; > uint32_t val; > bool wait = false; > @@ -1793,6 +1814,8 @@ static void intel_ddi_post_disable(struct intel_encoder > *intel_encoder, > > intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); > } > + > + intel_display_power_put(dev_priv, dig_port->ddi_io_power_domain); > } > > void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder, > @@ -2190,12 +2213,38 @@ void intel_ddi_init(struct drm_i915_private > *dev_priv, enum port port) > intel_encoder->get_hw_state = intel_ddi_get_hw_state; > intel_encoder->get_config = intel_ddi_get_config; > intel_encoder->suspend = intel_dp_encoder_suspend; > + intel_encoder->get_power_domains = intel_ddi_get_power_domains; > > intel_dig_port-