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-
[Intel-gfx] [PATCH 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL
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_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
(DDI_BUF_PORT_REVERSAL |
DDI_A_4_LANES);
+ switch (port) {
+
