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 <david.weineh...@linux.intel.com>
> Signed-off-by: Ander Conselvan de Oliveira 
> <ander.conselvan.de.olive...@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    | 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_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 585d149..0990107 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15487,6 +15487,18 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
>       else if (HAS_PCH_SPLIT(dev_priv))
>               ilk_wm_get_hw_state(dev);
>  
> +     for_each_intel_encoder(dev, encoder) {
> +             unsigned long long 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);
> +     }

intel_sanitize_encoder() could disable these power wells already, so the
corresponding reference needs to be taken before that.

> +
>       for_each_intel_crtc(dev, crtc) {
>               u64 put_domains;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index c8873c0..03447f8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -241,6 +241,8 @@ struct intel_encoder {
>        * be set correctly before calling this function. */
>       void (*get_config)(struct intel_encoder *,
>                          struct intel_crtc_state *pipe_config);
> +
> +     unsigned long long (*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
> @@ -1026,6 +1028,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 4d2dc31..67782d7 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -106,6 +106,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:
> @@ -402,18 +412,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 |         \
> @@ -468,12 +478,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) |                   \
> @@ -2154,26 +2164,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,
>       },
> @@ -2305,20 +2315,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 = &glk_ddi_io_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 = &glk_ddi_io_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 = &glk_ddi_io_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

Reply via email to