On Mon, 2017-02-27 at 22:38 +0200, Ville Syrjälä wrote: > On Fri, Feb 24, 2017 at 04:19: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. > > This one seems to kill SKL+MST. > > Just load the driver with an MST display plugged in and we get: > [ 79.389527] Kernel panic - not syncing: Timeout: Not all CPUs entered > broadcast exception handler > > So presumably we're trying to access something that's not powered/clocked > and the CPU just gets totally stuck.
Hmm, we need to enable the DDI IO power domain in the MST path too. /o\ I sent the fix as a separate series so CI doesn't get confused, although it doesn't seem to test MST+SKL. https://patchwork.freedesktop.org/series/20345/ Ander > > > > > v2: Rebase. > > - Take power domain references before sanitizing encoders. (Imre) > > - Add comment to get_encoder_power_domains() defition. (Ander) > > > > v3: Don't put the domain if called with HSW/BDW's analog encoder. (CI) > > > > v4: Put IO power domain before unmapping DPLL. (Imre) > > - Change return type of intel_ddi_get_power_domains() to u64. (Imre) > > > > Cc: David Weinehall <david.weineh...@linux.intel.com> > > Cc: Imre Deak <imre.d...@intel.com> > > Signed-off-by: Ander Conselvan de Oliveira > > <ander.conselvan.de.olive...@intel.com> > > Reviewed-by: David Weinehall <david.weineh...@linux.intel.com> # v1 > > Reviewed-by: Imre Deak <imre.d...@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 5 +++ > > drivers/gpu/drm/i915/intel_ddi.c | 49 ++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 4 ++ > > drivers/gpu/drm/i915/intel_runtime_pm.c | 68 > > +++++++++++++++++++-------------- > > 5 files changed, 117 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 4673912..5a032ba 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -344,6 +344,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..e9013f1 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1440,6 +1440,17 @@ bool intel_ddi_get_hw_state(struct intel_encoder > > *encoder, > > return ret; > > } > > > > +static u64 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 +1693,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 +1701,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 +1723,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 +1773,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; > > @@ -1782,6 +1802,9 @@ static void intel_ddi_post_disable(struct > > intel_encoder *intel_encoder, > > intel_edp_panel_off(intel_dp); > > } > > > > + if (dig_port) > > + intel_display_power_put(dev_priv, > > dig_port->ddi_io_power_domain); > > + > > if (IS_GEN9_BC(dev_priv)) > > I915_WRITE(DPLL_CTRL2, (I915_READ(DPLL_CTRL2) | > > DPLL_CTRL2_DDI_CLK_OFF(port))); > > @@ -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_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & > > (DDI_BUF_PORT_REVERSAL | > > DDI_A_4_LANES); > > > > + switch (port) { > > + case PORT_A: > > + intel_dig_port->ddi_io_power_domain = > > + POWER_DOMAIN_PORT_DDI_A_IO; > > + break; > > + case PORT_B: > > + intel_dig_port->ddi_io_power_domain = > > + POWER_DOMAIN_PORT_DDI_B_IO; > > + break; > > + case PORT_C: > > + intel_dig_port->ddi_io_power_domain = > > + POWER_DOMAIN_PORT_DDI_C_IO; > > + break; > > + case PORT_D: > > + intel_dig_port->ddi_io_power_domain = > > + POWER_DOMAIN_PORT_DDI_D_IO; > > + break; > > + case PORT_E: > > + intel_dig_port->ddi_io_power_domain = > > + POWER_DOMAIN_PORT_DDI_E_IO; > > + break; > > + default: > > + MISSING_CASE(port); > > + } > > + > > /* > > * Bspec says that DDI_A_4_LANES is the only supported configuration > > * for Broxton. Yet some BIOS fail to set this bit on port A if eDP > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 010086c..7673d5d 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -15413,6 +15413,24 @@ static void intel_modeset_readout_hw_state(struct > > drm_device *dev) > > } > > } > > > > +static void > > +get_encoder_power_domains(struct drm_i915_private *dev_priv) > > +{ > > + struct intel_encoder *encoder; > > + > > + for_each_intel_encoder(&dev_priv->drm, encoder) { > > + u64 get_domains; > > + enum intel_display_power_domain domain; > > + > > + if (!encoder->get_power_domains) > > + continue; > > + > > + get_domains = encoder->get_power_domains(encoder); > > + for_each_power_domain(domain, get_domains) > > + intel_display_power_get(dev_priv, domain); > > + } > > +} > > + > > /* Scan out the current hw modeset state, > > * and sanitizes it to the current state > > */ > > @@ -15428,6 +15446,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev) > > intel_modeset_readout_hw_state(dev); > > > > /* HW state is read out, now we need to sanitize this mess. */ > > + get_encoder_power_domains(dev_priv); > > + > > for_each_intel_encoder(dev, encoder) { > > intel_sanitize_encoder(encoder); > > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index 97621a1..0425dbe 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -241,6 +241,9 @@ struct intel_encoder { > > * be set correctly before calling this function. */ > > void (*get_config)(struct intel_encoder *, > > struct intel_crtc_state *pipe_config); > > + /* Returns a mask of power domains that need to be referenced as part > > + * of the hardware state readout code. */ > > + u64 (*get_power_domains)(struct intel_encoder *encoder); > > /* > > * Called during system suspend after all pending requests for the > > * encoder are flushed (for example for DP AUX transactions) and > > @@ -1027,6 +1030,7 @@ struct intel_digital_port { > > enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool); > > bool release_cl2_override; > > uint8_t max_lanes; > > + enum intel_display_power_domain ddi_io_power_domain; > > }; > > > > struct intel_dp_mst_encoder { > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 514ef56..012bc35 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -93,6 +93,16 @@ intel_display_power_domain_str(enum > > intel_display_power_domain domain) > > return "PORT_DDI_D_LANES"; > > case POWER_DOMAIN_PORT_DDI_E_LANES: > > return "PORT_DDI_E_LANES"; > > + case POWER_DOMAIN_PORT_DDI_A_IO: > > + return "PORT_DDI_A_IO"; > > + case POWER_DOMAIN_PORT_DDI_B_IO: > > + return "PORT_DDI_B_IO"; > > + case POWER_DOMAIN_PORT_DDI_C_IO: > > + return "PORT_DDI_C_IO"; > > + case POWER_DOMAIN_PORT_DDI_D_IO: > > + return "PORT_DDI_D_IO"; > > + case POWER_DOMAIN_PORT_DDI_E_IO: > > + return "PORT_DDI_E_IO"; > > case POWER_DOMAIN_PORT_DSI: > > return "PORT_DSI"; > > case POWER_DOMAIN_PORT_CRT: > > @@ -385,18 +395,18 @@ static void hsw_set_power_well(struct > > drm_i915_private *dev_priv, > > BIT_ULL(POWER_DOMAIN_AUDIO) | \ > > BIT_ULL(POWER_DOMAIN_VGA) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > -#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS ( \ > > - BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) | \ > > - BIT_ULL(POWER_DOMAIN_PORT_DDI_E_LANES) | \ > > +#define SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS ( \ > > + BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO) | \ > > + BIT_ULL(POWER_DOMAIN_PORT_DDI_E_IO) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > -#define SKL_DISPLAY_DDI_B_POWER_DOMAINS ( \ > > - BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) | \ > > +#define SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS ( \ > > + BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > -#define SKL_DISPLAY_DDI_C_POWER_DOMAINS ( \ > > - BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES) | \ > > +#define SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS ( \ > > + BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > -#define SKL_DISPLAY_DDI_D_POWER_DOMAINS ( \ > > - BIT_ULL(POWER_DOMAIN_PORT_DDI_D_LANES) | \ > > +#define SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS ( \ > > + BIT_ULL(POWER_DOMAIN_PORT_DDI_D_IO) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define SKL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \ > > @@ -451,12 +461,12 @@ static void hsw_set_power_well(struct > > drm_i915_private *dev_priv, > > BIT_ULL(POWER_DOMAIN_AUDIO) | \ > > BIT_ULL(POWER_DOMAIN_VGA) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > -#define GLK_DISPLAY_DDI_A_POWER_DOMAINS ( \ > > - BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES)) > > -#define GLK_DISPLAY_DDI_B_POWER_DOMAINS ( \ > > - BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES)) > > -#define GLK_DISPLAY_DDI_C_POWER_DOMAINS ( \ > > - BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES)) > > +#define GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS ( \ > > + BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO)) > > +#define GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS ( \ > > + BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO)) > > +#define GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS ( \ > > + BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO)) > > #define GLK_DPIO_CMN_A_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) | \ > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > @@ -2114,26 +2124,26 @@ static struct i915_power_well skl_power_wells[] = { > > .id = SKL_DISP_PW_2, > > }, > > { > > - .name = "DDI A/E power well", > > - .domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS, > > + .name = "DDI A/E IO power well", > > + .domains = SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > .id = SKL_DISP_PW_DDI_A_E, > > }, > > { > > - .name = "DDI B power well", > > - .domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS, > > + .name = "DDI B IO power well", > > + .domains = SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > .id = SKL_DISP_PW_DDI_B, > > }, > > { > > - .name = "DDI C power well", > > - .domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS, > > + .name = "DDI C IO power well", > > + .domains = SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > .id = SKL_DISP_PW_DDI_C, > > }, > > { > > - .name = "DDI D power well", > > - .domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS, > > + .name = "DDI D IO power well", > > + .domains = SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > .id = SKL_DISP_PW_DDI_D, > > }, > > @@ -2246,20 +2256,20 @@ static struct i915_power_well glk_power_wells[] = { > > .id = GLK_DISP_PW_AUX_C, > > }, > > { > > - .name = "DDI A power well", > > - .domains = GLK_DISPLAY_DDI_A_POWER_DOMAINS, > > + .name = "DDI A IO power well", > > + .domains = GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > .id = GLK_DISP_PW_DDI_A, > > }, > > { > > - .name = "DDI B power well", > > - .domains = GLK_DISPLAY_DDI_B_POWER_DOMAINS, > > + .name = "DDI B IO power well", > > + .domains = GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > .id = SKL_DISP_PW_DDI_B, > > }, > > { > > - .name = "DDI C power well", > > - .domains = GLK_DISPLAY_DDI_C_POWER_DOMAINS, > > + .name = "DDI C IO power well", > > + .domains = GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > .id = SKL_DISP_PW_DDI_C, > > }, > > -- > > 2.9.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
