* [PATCH 1/6] drm/i915: Store aux power domain in intel_dp
2017-02-10 13:29 [PATCH 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
@ 2017-02-10 13:29 ` Ander Conselvan de Oliveira
2017-02-16 9:05 ` Imre Deak
2017-02-10 13:29 ` [PATCH 2/6] drm/i915: Store encoder power domain in struct intel_encoder Ander Conselvan de Oliveira
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-10 13:29 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
The aux power domain only makes sense in the DP code. Storing it in
struct intel_dp avoids some indirection.
v2: Rebase
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 50 -----------------------------
drivers/gpu/drm/i915/intel_dp.c | 61 ++++++++++++++----------------------
drivers/gpu/drm/i915/intel_drv.h | 3 +-
3 files changed, 24 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ab78df9..1e70776 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5589,26 +5589,6 @@ static enum intel_display_power_domain port_to_power_domain(enum port port)
}
}
-static enum intel_display_power_domain port_to_aux_power_domain(enum port port)
-{
- switch (port) {
- case PORT_A:
- return POWER_DOMAIN_AUX_A;
- case PORT_B:
- return POWER_DOMAIN_AUX_B;
- case PORT_C:
- return POWER_DOMAIN_AUX_C;
- case PORT_D:
- return POWER_DOMAIN_AUX_D;
- case PORT_E:
- /* FIXME: Check VBT for actual wiring of PORT E */
- return POWER_DOMAIN_AUX_D;
- default:
- MISSING_CASE(port);
- return POWER_DOMAIN_AUX_A;
- }
-}
-
enum intel_display_power_domain
intel_display_port_power_domain(struct intel_encoder *intel_encoder)
{
@@ -5636,36 +5616,6 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder)
}
}
-enum intel_display_power_domain
-intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder)
-{
- struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
- struct intel_digital_port *intel_dig_port;
-
- switch (intel_encoder->type) {
- case INTEL_OUTPUT_UNKNOWN:
- case INTEL_OUTPUT_HDMI:
- /*
- * Only DDI platforms should ever use these output types.
- * We can get here after the HDMI detect code has already set
- * the type of the shared encoder. Since we can't be sure
- * what's the status of the given connectors, play safe and
- * run the DP detection too.
- */
- WARN_ON_ONCE(!HAS_DDI(dev_priv));
- case INTEL_OUTPUT_DP:
- case INTEL_OUTPUT_EDP:
- intel_dig_port = enc_to_dig_port(&intel_encoder->base);
- return port_to_aux_power_domain(intel_dig_port->port);
- case INTEL_OUTPUT_DP_MST:
- intel_dig_port = enc_to_mst(&intel_encoder->base)->primary;
- return port_to_aux_power_domain(intel_dig_port->port);
- default:
- MISSING_CASE(intel_encoder->type);
- return POWER_DOMAIN_AUX_A;
- }
-}
-
static u64 get_crtc_power_domains(struct drm_crtc *crtc,
struct intel_crtc_state *crtc_state)
{
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fa77e96..e2e4766 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -396,14 +396,12 @@ static void pps_lock(struct intel_dp *intel_dp)
struct intel_encoder *encoder = &intel_dig_port->base;
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- enum intel_display_power_domain power_domain;
/*
* See vlv_power_sequencer_reset() why we need
* a power domain reference here.
*/
- power_domain = intel_display_port_aux_power_domain(encoder);
- intel_display_power_get(dev_priv, power_domain);
+ intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
mutex_lock(&dev_priv->pps_mutex);
}
@@ -414,12 +412,10 @@ static void pps_unlock(struct intel_dp *intel_dp)
struct intel_encoder *encoder = &intel_dig_port->base;
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- enum intel_display_power_domain power_domain;
mutex_unlock(&dev_priv->pps_mutex);
- power_domain = intel_display_port_aux_power_domain(encoder);
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
}
static void
@@ -2005,9 +2001,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct drm_i915_private *dev_priv = to_i915(dev);
- enum intel_display_power_domain power_domain;
u32 pp;
i915_reg_t pp_stat_reg, pp_ctrl_reg;
bool need_to_disable = !intel_dp->want_panel_vdd;
@@ -2023,8 +2017,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
if (edp_have_panel_vdd(intel_dp))
return need_to_disable;
- power_domain = intel_display_port_aux_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
+ intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
DRM_DEBUG_KMS("Turning eDP port %c VDD on\n",
port_name(intel_dig_port->port));
@@ -2082,8 +2075,6 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_port *intel_dig_port =
dp_to_dig_port(intel_dp);
- struct intel_encoder *intel_encoder = &intel_dig_port->base;
- enum intel_display_power_domain power_domain;
u32 pp;
i915_reg_t pp_stat_reg, pp_ctrl_reg;
@@ -2113,8 +2104,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
if ((pp & PANEL_POWER_ON) == 0)
intel_dp->panel_power_off_time = ktime_get_boottime();
- power_domain = intel_display_port_aux_power_domain(intel_encoder);
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
}
static void edp_panel_vdd_work(struct work_struct *__work)
@@ -2227,11 +2217,8 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
static void edp_panel_off(struct intel_dp *intel_dp)
{
- struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct drm_i915_private *dev_priv = to_i915(dev);
- enum intel_display_power_domain power_domain;
u32 pp;
i915_reg_t pp_ctrl_reg;
@@ -2263,8 +2250,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
- power_domain = intel_display_port_aux_power_domain(intel_encoder);
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
}
void intel_edp_panel_off(struct intel_dp *intel_dp)
@@ -4587,11 +4573,9 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct drm_device *dev = connector->dev;
enum drm_connector_status status;
- enum intel_display_power_domain power_domain;
u8 sink_irq_vector = 0;
- power_domain = intel_display_port_aux_power_domain(intel_encoder);
- intel_display_power_get(to_i915(dev), power_domain);
+ intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
/* Can't disconnect eDP, but you can close the lid... */
if (is_edp(intel_dp))
@@ -4688,7 +4672,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
if (status != connector_status_connected && !intel_dp->is_mst)
intel_dp_unset_edid(intel_dp);
- intel_display_power_put(to_i915(dev), power_domain);
+ intel_display_power_put(to_i915(dev), intel_dp->aux_power_domain);
return status;
}
@@ -4716,7 +4700,6 @@ intel_dp_force(struct drm_connector *connector)
struct intel_dp *intel_dp = intel_attached_dp(connector);
struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
- enum intel_display_power_domain power_domain;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
connector->base.id, connector->name);
@@ -4725,12 +4708,11 @@ intel_dp_force(struct drm_connector *connector)
if (connector->status != connector_status_connected)
return;
- power_domain = intel_display_port_aux_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
+ intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
intel_dp_set_edid(intel_dp);
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
if (intel_encoder->type != INTEL_OUTPUT_EDP)
intel_encoder->type = INTEL_OUTPUT_DP;
@@ -4965,7 +4947,6 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- enum intel_display_power_domain power_domain;
lockdep_assert_held(&dev_priv->pps_mutex);
@@ -4979,8 +4960,7 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
* indefinitely.
*/
DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n");
- power_domain = intel_display_port_aux_power_domain(&intel_dig_port->base);
- intel_display_power_get(dev_priv, power_domain);
+ intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
edp_panel_vdd_schedule_off(intel_dp);
}
@@ -5052,10 +5032,8 @@ enum irqreturn
intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
{
struct intel_dp *intel_dp = &intel_dig_port->dp;
- struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- enum intel_display_power_domain power_domain;
enum irqreturn ret = IRQ_NONE;
if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
@@ -5083,8 +5061,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
return IRQ_NONE;
}
- power_domain = intel_display_port_aux_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
+ intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
if (intel_dp->is_mst) {
if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
@@ -5112,7 +5089,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
ret = IRQ_HANDLED;
put_power:
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
return ret;
}
@@ -5903,27 +5880,35 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
return false;
}
+/* Set up the hotplug pin and aux power domain. */
static void
intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
{
struct intel_encoder *encoder = &intel_dig_port->base;
+ struct intel_dp *intel_dp = &intel_dig_port->dp;
- /* Set up the hotplug pin. */
switch (intel_dig_port->port) {
case PORT_A:
encoder->hpd_pin = HPD_PORT_A;
+ intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A;
break;
case PORT_B:
encoder->hpd_pin = HPD_PORT_B;
+ intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B;
break;
case PORT_C:
encoder->hpd_pin = HPD_PORT_C;
+ intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C;
break;
case PORT_D:
encoder->hpd_pin = HPD_PORT_D;
+ intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
break;
case PORT_E:
encoder->hpd_pin = HPD_PORT_E;
+
+ /* FIXME: Check VBT for actual wiring of PORT E */
+ intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
break;
default:
MISSING_CASE(intel_dig_port->port);
@@ -6003,6 +5988,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
connector->interlace_allowed = true;
connector->doublescan_allowed = 0;
+ intel_dp_init_connector_port_info(intel_dig_port);
+
intel_dp_aux_init(intel_dp);
INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
@@ -6015,8 +6002,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
else
intel_connector->get_hw_state = intel_connector_get_hw_state;
- intel_dp_init_connector_port_info(intel_dig_port);
-
/* init MST on ports that can support it */
if (HAS_DP_MST(dev_priv) && !is_edp(intel_dp) &&
(port == PORT_B || port == PORT_C || port == PORT_D))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6e37fba..3ddeaa6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -950,6 +950,7 @@ struct intel_dp {
/* sink or branch descriptor */
struct intel_dp_desc desc;
struct drm_dp_aux aux;
+ enum intel_display_power_domain aux_power_domain;
uint8_t train_set[4];
int panel_power_up_delay;
int panel_power_down_delay;
@@ -1421,8 +1422,6 @@ void hsw_enable_ips(struct intel_crtc *crtc);
void hsw_disable_ips(struct intel_crtc *crtc);
enum intel_display_power_domain
intel_display_port_power_domain(struct intel_encoder *intel_encoder);
-enum intel_display_power_domain
-intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder);
void intel_mode_from_pipe_config(struct drm_display_mode *mode,
struct intel_crtc_state *pipe_config);
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] drm/i915: Store aux power domain in intel_dp
2017-02-10 13:29 ` [PATCH 1/6] drm/i915: Store aux power domain in intel_dp Ander Conselvan de Oliveira
@ 2017-02-16 9:05 ` Imre Deak
2017-02-16 12:23 ` Ander Conselvan De Oliveira
0 siblings, 1 reply; 17+ messages in thread
From: Imre Deak @ 2017-02-16 9:05 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
On Fri, Feb 10, 2017 at 03:29:54PM +0200, Ander Conselvan de Oliveira wrote:
> The aux power domain only makes sense in the DP code. Storing it in
> struct intel_dp avoids some indirection.
>
> v2: Rebase
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 50 -----------------------------
> drivers/gpu/drm/i915/intel_dp.c | 61 ++++++++++++++----------------------
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> 3 files changed, 24 insertions(+), 90 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ab78df9..1e70776 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5589,26 +5589,6 @@ static enum intel_display_power_domain port_to_power_domain(enum port port)
> }
> }
>
> -static enum intel_display_power_domain port_to_aux_power_domain(enum port port)
> -{
> - switch (port) {
> - case PORT_A:
> - return POWER_DOMAIN_AUX_A;
> - case PORT_B:
> - return POWER_DOMAIN_AUX_B;
> - case PORT_C:
> - return POWER_DOMAIN_AUX_C;
> - case PORT_D:
> - return POWER_DOMAIN_AUX_D;
> - case PORT_E:
> - /* FIXME: Check VBT for actual wiring of PORT E */
> - return POWER_DOMAIN_AUX_D;
> - default:
> - MISSING_CASE(port);
> - return POWER_DOMAIN_AUX_A;
> - }
> -}
> -
> enum intel_display_power_domain
> intel_display_port_power_domain(struct intel_encoder *intel_encoder)
> {
> @@ -5636,36 +5616,6 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder)
> }
> }
>
> -enum intel_display_power_domain
> -intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder)
> -{
> - struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
> - struct intel_digital_port *intel_dig_port;
> -
> - switch (intel_encoder->type) {
> - case INTEL_OUTPUT_UNKNOWN:
> - case INTEL_OUTPUT_HDMI:
> - /*
> - * Only DDI platforms should ever use these output types.
> - * We can get here after the HDMI detect code has already set
> - * the type of the shared encoder. Since we can't be sure
> - * what's the status of the given connectors, play safe and
> - * run the DP detection too.
> - */
> - WARN_ON_ONCE(!HAS_DDI(dev_priv));
> - case INTEL_OUTPUT_DP:
> - case INTEL_OUTPUT_EDP:
> - intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> - return port_to_aux_power_domain(intel_dig_port->port);
> - case INTEL_OUTPUT_DP_MST:
> - intel_dig_port = enc_to_mst(&intel_encoder->base)->primary;
> - return port_to_aux_power_domain(intel_dig_port->port);
I was wondering where the dp->aux_power_domain gets inited for the MST case,
but looks like this case could never happen even now. The patch looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>
> - default:
> - MISSING_CASE(intel_encoder->type);
> - return POWER_DOMAIN_AUX_A;
> - }
> -}
> -
> static u64 get_crtc_power_domains(struct drm_crtc *crtc,
> struct intel_crtc_state *crtc_state)
> {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fa77e96..e2e4766 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -396,14 +396,12 @@ static void pps_lock(struct intel_dp *intel_dp)
> struct intel_encoder *encoder = &intel_dig_port->base;
> struct drm_device *dev = encoder->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - enum intel_display_power_domain power_domain;
>
> /*
> * See vlv_power_sequencer_reset() why we need
> * a power domain reference here.
> */
> - power_domain = intel_display_port_aux_power_domain(encoder);
> - intel_display_power_get(dev_priv, power_domain);
> + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>
> mutex_lock(&dev_priv->pps_mutex);
> }
> @@ -414,12 +412,10 @@ static void pps_unlock(struct intel_dp *intel_dp)
> struct intel_encoder *encoder = &intel_dig_port->base;
> struct drm_device *dev = encoder->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - enum intel_display_power_domain power_domain;
>
> mutex_unlock(&dev_priv->pps_mutex);
>
> - power_domain = intel_display_port_aux_power_domain(encoder);
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> }
>
> static void
> @@ -2005,9 +2001,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - enum intel_display_power_domain power_domain;
> u32 pp;
> i915_reg_t pp_stat_reg, pp_ctrl_reg;
> bool need_to_disable = !intel_dp->want_panel_vdd;
> @@ -2023,8 +2017,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
> if (edp_have_panel_vdd(intel_dp))
> return need_to_disable;
>
> - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> - intel_display_power_get(dev_priv, power_domain);
> + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>
> DRM_DEBUG_KMS("Turning eDP port %c VDD on\n",
> port_name(intel_dig_port->port));
> @@ -2082,8 +2075,6 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> - enum intel_display_power_domain power_domain;
> u32 pp;
> i915_reg_t pp_stat_reg, pp_ctrl_reg;
>
> @@ -2113,8 +2104,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
> if ((pp & PANEL_POWER_ON) == 0)
> intel_dp->panel_power_off_time = ktime_get_boottime();
>
> - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> }
>
> static void edp_panel_vdd_work(struct work_struct *__work)
> @@ -2227,11 +2217,8 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
>
> static void edp_panel_off(struct intel_dp *intel_dp)
> {
> - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = to_i915(dev);
> - enum intel_display_power_domain power_domain;
> u32 pp;
> i915_reg_t pp_ctrl_reg;
>
> @@ -2263,8 +2250,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
> wait_panel_off(intel_dp);
>
> /* We got a reference when we enabled the VDD. */
> - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> }
>
> void intel_edp_panel_off(struct intel_dp *intel_dp)
> @@ -4587,11 +4573,9 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> struct intel_encoder *intel_encoder = &intel_dig_port->base;
> struct drm_device *dev = connector->dev;
> enum drm_connector_status status;
> - enum intel_display_power_domain power_domain;
> u8 sink_irq_vector = 0;
>
> - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> - intel_display_power_get(to_i915(dev), power_domain);
> + intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
>
> /* Can't disconnect eDP, but you can close the lid... */
> if (is_edp(intel_dp))
> @@ -4688,7 +4672,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> if (status != connector_status_connected && !intel_dp->is_mst)
> intel_dp_unset_edid(intel_dp);
>
> - intel_display_power_put(to_i915(dev), power_domain);
> + intel_display_power_put(to_i915(dev), intel_dp->aux_power_domain);
> return status;
> }
>
> @@ -4716,7 +4700,6 @@ intel_dp_force(struct drm_connector *connector)
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
> struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
> - enum intel_display_power_domain power_domain;
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> connector->base.id, connector->name);
> @@ -4725,12 +4708,11 @@ intel_dp_force(struct drm_connector *connector)
> if (connector->status != connector_status_connected)
> return;
>
> - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> - intel_display_power_get(dev_priv, power_domain);
> + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>
> intel_dp_set_edid(intel_dp);
>
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
>
> if (intel_encoder->type != INTEL_OUTPUT_EDP)
> intel_encoder->type = INTEL_OUTPUT_DP;
> @@ -4965,7 +4947,6 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - enum intel_display_power_domain power_domain;
>
> lockdep_assert_held(&dev_priv->pps_mutex);
>
> @@ -4979,8 +4960,7 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
> * indefinitely.
> */
> DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n");
> - power_domain = intel_display_port_aux_power_domain(&intel_dig_port->base);
> - intel_display_power_get(dev_priv, power_domain);
> + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>
> edp_panel_vdd_schedule_off(intel_dp);
> }
> @@ -5052,10 +5032,8 @@ enum irqreturn
> intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> {
> struct intel_dp *intel_dp = &intel_dig_port->dp;
> - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - enum intel_display_power_domain power_domain;
> enum irqreturn ret = IRQ_NONE;
>
> if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
> @@ -5083,8 +5061,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> return IRQ_NONE;
> }
>
> - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> - intel_display_power_get(dev_priv, power_domain);
> + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>
> if (intel_dp->is_mst) {
> if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
> @@ -5112,7 +5089,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> ret = IRQ_HANDLED;
>
> put_power:
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
>
> return ret;
> }
> @@ -5903,27 +5880,35 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> return false;
> }
>
> +/* Set up the hotplug pin and aux power domain. */
> static void
> intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
> {
> struct intel_encoder *encoder = &intel_dig_port->base;
> + struct intel_dp *intel_dp = &intel_dig_port->dp;
>
> - /* Set up the hotplug pin. */
> switch (intel_dig_port->port) {
> case PORT_A:
> encoder->hpd_pin = HPD_PORT_A;
> + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A;
> break;
> case PORT_B:
> encoder->hpd_pin = HPD_PORT_B;
> + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B;
> break;
> case PORT_C:
> encoder->hpd_pin = HPD_PORT_C;
> + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C;
> break;
> case PORT_D:
> encoder->hpd_pin = HPD_PORT_D;
> + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
> break;
> case PORT_E:
> encoder->hpd_pin = HPD_PORT_E;
> +
> + /* FIXME: Check VBT for actual wiring of PORT E */
> + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
> break;
> default:
> MISSING_CASE(intel_dig_port->port);
> @@ -6003,6 +5988,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> connector->interlace_allowed = true;
> connector->doublescan_allowed = 0;
>
> + intel_dp_init_connector_port_info(intel_dig_port);
> +
> intel_dp_aux_init(intel_dp);
>
> INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
> @@ -6015,8 +6002,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> else
> intel_connector->get_hw_state = intel_connector_get_hw_state;
>
> - intel_dp_init_connector_port_info(intel_dig_port);
> -
> /* init MST on ports that can support it */
> if (HAS_DP_MST(dev_priv) && !is_edp(intel_dp) &&
> (port == PORT_B || port == PORT_C || port == PORT_D))
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6e37fba..3ddeaa6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -950,6 +950,7 @@ struct intel_dp {
> /* sink or branch descriptor */
> struct intel_dp_desc desc;
> struct drm_dp_aux aux;
> + enum intel_display_power_domain aux_power_domain;
> uint8_t train_set[4];
> int panel_power_up_delay;
> int panel_power_down_delay;
> @@ -1421,8 +1422,6 @@ void hsw_enable_ips(struct intel_crtc *crtc);
> void hsw_disable_ips(struct intel_crtc *crtc);
> enum intel_display_power_domain
> intel_display_port_power_domain(struct intel_encoder *intel_encoder);
> -enum intel_display_power_domain
> -intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder);
> void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> struct intel_crtc_state *pipe_config);
>
> --
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] drm/i915: Store aux power domain in intel_dp
2017-02-16 9:05 ` Imre Deak
@ 2017-02-16 12:23 ` Ander Conselvan De Oliveira
2017-02-16 12:28 ` Jani Nikula
0 siblings, 1 reply; 17+ messages in thread
From: Ander Conselvan De Oliveira @ 2017-02-16 12:23 UTC (permalink / raw)
To: imre.deak; +Cc: intel-gfx
On Thu, 2017-02-16 at 11:05 +0200, Imre Deak wrote:
> On Fri, Feb 10, 2017 at 03:29:54PM +0200, Ander Conselvan de Oliveira wrote:
> > The aux power domain only makes sense in the DP code. Storing it in
> > struct intel_dp avoids some indirection.
> >
> > v2: Rebase
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 50 -----------------------------
> > drivers/gpu/drm/i915/intel_dp.c | 61 ++++++++++++++----------------------
> > drivers/gpu/drm/i915/intel_drv.h | 3 +-
> > 3 files changed, 24 insertions(+), 90 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ab78df9..1e70776 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5589,26 +5589,6 @@ static enum intel_display_power_domain port_to_power_domain(enum port port)
> > }
> > }
> >
> > -static enum intel_display_power_domain port_to_aux_power_domain(enum port port)
> > -{
> > - switch (port) {
> > - case PORT_A:
> > - return POWER_DOMAIN_AUX_A;
> > - case PORT_B:
> > - return POWER_DOMAIN_AUX_B;
> > - case PORT_C:
> > - return POWER_DOMAIN_AUX_C;
> > - case PORT_D:
> > - return POWER_DOMAIN_AUX_D;
> > - case PORT_E:
> > - /* FIXME: Check VBT for actual wiring of PORT E */
> > - return POWER_DOMAIN_AUX_D;
> > - default:
> > - MISSING_CASE(port);
> > - return POWER_DOMAIN_AUX_A;
> > - }
> > -}
> > -
> > enum intel_display_power_domain
> > intel_display_port_power_domain(struct intel_encoder *intel_encoder)
> > {
> > @@ -5636,36 +5616,6 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder)
> > }
> > }
> >
> > -enum intel_display_power_domain
> > -intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder)
> > -{
> > - struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
> > - struct intel_digital_port *intel_dig_port;
> > -
> > - switch (intel_encoder->type) {
> > - case INTEL_OUTPUT_UNKNOWN:
> > - case INTEL_OUTPUT_HDMI:
> > - /*
> > - * Only DDI platforms should ever use these output types.
> > - * We can get here after the HDMI detect code has already set
> > - * the type of the shared encoder. Since we can't be sure
> > - * what's the status of the given connectors, play safe and
> > - * run the DP detection too.
> > - */
> > - WARN_ON_ONCE(!HAS_DDI(dev_priv));
> > - case INTEL_OUTPUT_DP:
> > - case INTEL_OUTPUT_EDP:
> > - intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> > - return port_to_aux_power_domain(intel_dig_port->port);
> > - case INTEL_OUTPUT_DP_MST:
> > - intel_dig_port = enc_to_mst(&intel_encoder->base)->primary;
> > - return port_to_aux_power_domain(intel_dig_port->port);
>
> I was wondering where the dp->aux_power_domain gets inited for the MST case,
> but looks like this case could never happen even now. The patch looks ok:
Looks like I forgot to Cc Jani, but he had the same concern: https://lists.freed
esktop.org/archives/intel-gfx/2017-February/119027.html
The important part is that when the MST connector is initialized, it receives a
struct intel_dp as a parameter, and that struct has the aux power domain
initialized. That is what ends up in the ->primary field which the function
above uses.
> Reviewed-by: Imre Deak <imre.deak@intel.com>
Thanks,
Ander
>
> > - default:
> > - MISSING_CASE(intel_encoder->type);
> > - return POWER_DOMAIN_AUX_A;
> > - }
> > -}
> > -
> > static u64 get_crtc_power_domains(struct drm_crtc *crtc,
> > struct intel_crtc_state *crtc_state)
> > {
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index fa77e96..e2e4766 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -396,14 +396,12 @@ static void pps_lock(struct intel_dp *intel_dp)
> > struct intel_encoder *encoder = &intel_dig_port->base;
> > struct drm_device *dev = encoder->base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > - enum intel_display_power_domain power_domain;
> >
> > /*
> > * See vlv_power_sequencer_reset() why we need
> > * a power domain reference here.
> > */
> > - power_domain = intel_display_port_aux_power_domain(encoder);
> > - intel_display_power_get(dev_priv, power_domain);
> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> >
> > mutex_lock(&dev_priv->pps_mutex);
> > }
> > @@ -414,12 +412,10 @@ static void pps_unlock(struct intel_dp *intel_dp)
> > struct intel_encoder *encoder = &intel_dig_port->base;
> > struct drm_device *dev = encoder->base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > - enum intel_display_power_domain power_domain;
> >
> > mutex_unlock(&dev_priv->pps_mutex);
> >
> > - power_domain = intel_display_port_aux_power_domain(encoder);
> > - intel_display_power_put(dev_priv, power_domain);
> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > }
> >
> > static void
> > @@ -2005,9 +2001,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
> > {
> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > - enum intel_display_power_domain power_domain;
> > u32 pp;
> > i915_reg_t pp_stat_reg, pp_ctrl_reg;
> > bool need_to_disable = !intel_dp->want_panel_vdd;
> > @@ -2023,8 +2017,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
> > if (edp_have_panel_vdd(intel_dp))
> > return need_to_disable;
> >
> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> > - intel_display_power_get(dev_priv, power_domain);
> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> >
> > DRM_DEBUG_KMS("Turning eDP port %c VDD on\n",
> > port_name(intel_dig_port->port));
> > @@ -2082,8 +2075,6 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > - enum intel_display_power_domain power_domain;
> > u32 pp;
> > i915_reg_t pp_stat_reg, pp_ctrl_reg;
> >
> > @@ -2113,8 +2104,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
> > if ((pp & PANEL_POWER_ON) == 0)
> > intel_dp->panel_power_off_time = ktime_get_boottime();
> >
> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> > - intel_display_power_put(dev_priv, power_domain);
> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > }
> >
> > static void edp_panel_vdd_work(struct work_struct *__work)
> > @@ -2227,11 +2217,8 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
> >
> > static void edp_panel_off(struct intel_dp *intel_dp)
> > {
> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > - enum intel_display_power_domain power_domain;
> > u32 pp;
> > i915_reg_t pp_ctrl_reg;
> >
> > @@ -2263,8 +2250,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
> > wait_panel_off(intel_dp);
> >
> > /* We got a reference when we enabled the VDD. */
> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> > - intel_display_power_put(dev_priv, power_domain);
> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > }
> >
> > void intel_edp_panel_off(struct intel_dp *intel_dp)
> > @@ -4587,11 +4573,9 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> > struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > struct drm_device *dev = connector->dev;
> > enum drm_connector_status status;
> > - enum intel_display_power_domain power_domain;
> > u8 sink_irq_vector = 0;
> >
> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> > - intel_display_power_get(to_i915(dev), power_domain);
> > + intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
> >
> > /* Can't disconnect eDP, but you can close the lid... */
> > if (is_edp(intel_dp))
> > @@ -4688,7 +4672,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> > if (status != connector_status_connected && !intel_dp->is_mst)
> > intel_dp_unset_edid(intel_dp);
> >
> > - intel_display_power_put(to_i915(dev), power_domain);
> > + intel_display_power_put(to_i915(dev), intel_dp->aux_power_domain);
> > return status;
> > }
> >
> > @@ -4716,7 +4700,6 @@ intel_dp_force(struct drm_connector *connector)
> > struct intel_dp *intel_dp = intel_attached_dp(connector);
> > struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
> > struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
> > - enum intel_display_power_domain power_domain;
> >
> > DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > connector->base.id, connector->name);
> > @@ -4725,12 +4708,11 @@ intel_dp_force(struct drm_connector *connector)
> > if (connector->status != connector_status_connected)
> > return;
> >
> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> > - intel_display_power_get(dev_priv, power_domain);
> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> >
> > intel_dp_set_edid(intel_dp);
> >
> > - intel_display_power_put(dev_priv, power_domain);
> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> >
> > if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > intel_encoder->type = INTEL_OUTPUT_DP;
> > @@ -4965,7 +4947,6 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > struct drm_device *dev = intel_dig_port->base.base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > - enum intel_display_power_domain power_domain;
> >
> > lockdep_assert_held(&dev_priv->pps_mutex);
> >
> > @@ -4979,8 +4960,7 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
> > * indefinitely.
> > */
> > DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n");
> > - power_domain = intel_display_port_aux_power_domain(&intel_dig_port->base);
> > - intel_display_power_get(dev_priv, power_domain);
> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> >
> > edp_panel_vdd_schedule_off(intel_dp);
> > }
> > @@ -5052,10 +5032,8 @@ enum irqreturn
> > intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> > {
> > struct intel_dp *intel_dp = &intel_dig_port->dp;
> > - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > struct drm_device *dev = intel_dig_port->base.base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > - enum intel_display_power_domain power_domain;
> > enum irqreturn ret = IRQ_NONE;
> >
> > if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
> > @@ -5083,8 +5061,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> > return IRQ_NONE;
> > }
> >
> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
> > - intel_display_power_get(dev_priv, power_domain);
> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> >
> > if (intel_dp->is_mst) {
> > if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
> > @@ -5112,7 +5089,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> > ret = IRQ_HANDLED;
> >
> > put_power:
> > - intel_display_power_put(dev_priv, power_domain);
> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> >
> > return ret;
> > }
> > @@ -5903,27 +5880,35 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > return false;
> > }
> >
> > +/* Set up the hotplug pin and aux power domain. */
> > static void
> > intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
> > {
> > struct intel_encoder *encoder = &intel_dig_port->base;
> > + struct intel_dp *intel_dp = &intel_dig_port->dp;
> >
> > - /* Set up the hotplug pin. */
> > switch (intel_dig_port->port) {
> > case PORT_A:
> > encoder->hpd_pin = HPD_PORT_A;
> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A;
> > break;
> > case PORT_B:
> > encoder->hpd_pin = HPD_PORT_B;
> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B;
> > break;
> > case PORT_C:
> > encoder->hpd_pin = HPD_PORT_C;
> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C;
> > break;
> > case PORT_D:
> > encoder->hpd_pin = HPD_PORT_D;
> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
> > break;
> > case PORT_E:
> > encoder->hpd_pin = HPD_PORT_E;
> > +
> > + /* FIXME: Check VBT for actual wiring of PORT E */
> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
> > break;
> > default:
> > MISSING_CASE(intel_dig_port->port);
> > @@ -6003,6 +5988,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> > connector->interlace_allowed = true;
> > connector->doublescan_allowed = 0;
> >
> > + intel_dp_init_connector_port_info(intel_dig_port);
> > +
> > intel_dp_aux_init(intel_dp);
> >
> > INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
> > @@ -6015,8 +6002,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> > else
> > intel_connector->get_hw_state = intel_connector_get_hw_state;
> >
> > - intel_dp_init_connector_port_info(intel_dig_port);
> > -
> > /* init MST on ports that can support it */
> > if (HAS_DP_MST(dev_priv) && !is_edp(intel_dp) &&
> > (port == PORT_B || port == PORT_C || port == PORT_D))
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 6e37fba..3ddeaa6 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -950,6 +950,7 @@ struct intel_dp {
> > /* sink or branch descriptor */
> > struct intel_dp_desc desc;
> > struct drm_dp_aux aux;
> > + enum intel_display_power_domain aux_power_domain;
> > uint8_t train_set[4];
> > int panel_power_up_delay;
> > int panel_power_down_delay;
> > @@ -1421,8 +1422,6 @@ void hsw_enable_ips(struct intel_crtc *crtc);
> > void hsw_disable_ips(struct intel_crtc *crtc);
> > enum intel_display_power_domain
> > intel_display_port_power_domain(struct intel_encoder *intel_encoder);
> > -enum intel_display_power_domain
> > -intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder);
> > void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> > struct intel_crtc_state *pipe_config);
> >
> > --
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] drm/i915: Store aux power domain in intel_dp
2017-02-16 12:23 ` Ander Conselvan De Oliveira
@ 2017-02-16 12:28 ` Jani Nikula
0 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2017-02-16 12:28 UTC (permalink / raw)
To: Ander Conselvan De Oliveira, imre.deak; +Cc: intel-gfx
On Thu, 16 Feb 2017, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote:
> On Thu, 2017-02-16 at 11:05 +0200, Imre Deak wrote:
>> On Fri, Feb 10, 2017 at 03:29:54PM +0200, Ander Conselvan de Oliveira wrote:
>> > The aux power domain only makes sense in the DP code. Storing it in
>> > struct intel_dp avoids some indirection.
>> >
>> > v2: Rebase
>> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/intel_display.c | 50 -----------------------------
>> > drivers/gpu/drm/i915/intel_dp.c | 61 ++++++++++++++----------------------
>> > drivers/gpu/drm/i915/intel_drv.h | 3 +-
>> > 3 files changed, 24 insertions(+), 90 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > index ab78df9..1e70776 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -5589,26 +5589,6 @@ static enum intel_display_power_domain port_to_power_domain(enum port port)
>> > }
>> > }
>> >
>> > -static enum intel_display_power_domain port_to_aux_power_domain(enum port port)
>> > -{
>> > - switch (port) {
>> > - case PORT_A:
>> > - return POWER_DOMAIN_AUX_A;
>> > - case PORT_B:
>> > - return POWER_DOMAIN_AUX_B;
>> > - case PORT_C:
>> > - return POWER_DOMAIN_AUX_C;
>> > - case PORT_D:
>> > - return POWER_DOMAIN_AUX_D;
>> > - case PORT_E:
>> > - /* FIXME: Check VBT for actual wiring of PORT E */
>> > - return POWER_DOMAIN_AUX_D;
>> > - default:
>> > - MISSING_CASE(port);
>> > - return POWER_DOMAIN_AUX_A;
>> > - }
>> > -}
>> > -
>> > enum intel_display_power_domain
>> > intel_display_port_power_domain(struct intel_encoder *intel_encoder)
>> > {
>> > @@ -5636,36 +5616,6 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder)
>> > }
>> > }
>> >
>> > -enum intel_display_power_domain
>> > -intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder)
>> > -{
>> > - struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
>> > - struct intel_digital_port *intel_dig_port;
>> > -
>> > - switch (intel_encoder->type) {
>> > - case INTEL_OUTPUT_UNKNOWN:
>> > - case INTEL_OUTPUT_HDMI:
>> > - /*
>> > - * Only DDI platforms should ever use these output types.
>> > - * We can get here after the HDMI detect code has already set
>> > - * the type of the shared encoder. Since we can't be sure
>> > - * what's the status of the given connectors, play safe and
>> > - * run the DP detection too.
>> > - */
>> > - WARN_ON_ONCE(!HAS_DDI(dev_priv));
>> > - case INTEL_OUTPUT_DP:
>> > - case INTEL_OUTPUT_EDP:
>> > - intel_dig_port = enc_to_dig_port(&intel_encoder->base);
>> > - return port_to_aux_power_domain(intel_dig_port->port);
>> > - case INTEL_OUTPUT_DP_MST:
>> > - intel_dig_port = enc_to_mst(&intel_encoder->base)->primary;
>> > - return port_to_aux_power_domain(intel_dig_port->port);
>>
>> I was wondering where the dp->aux_power_domain gets inited for the MST case,
>> but looks like this case could never happen even now. The patch looks ok:
>
> Looks like I forgot to Cc Jani, but he had the same concern: https://lists.freed
> esktop.org/archives/intel-gfx/2017-February/119027.html
Yeah, sorry I failed to follow-up here. /o\
>
> The important part is that when the MST connector is initialized, it receives a
> struct intel_dp as a parameter, and that struct has the aux power domain
> initialized. That is what ends up in the ->primary field which the function
> above uses.
>
>> Reviewed-by: Imre Deak <imre.deak@intel.com>
>
> Thanks,
> Ander
>
>>
>> > - default:
>> > - MISSING_CASE(intel_encoder->type);
>> > - return POWER_DOMAIN_AUX_A;
>> > - }
>> > -}
>> > -
>> > static u64 get_crtc_power_domains(struct drm_crtc *crtc,
>> > struct intel_crtc_state *crtc_state)
>> > {
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > index fa77e96..e2e4766 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -396,14 +396,12 @@ static void pps_lock(struct intel_dp *intel_dp)
>> > struct intel_encoder *encoder = &intel_dig_port->base;
>> > struct drm_device *dev = encoder->base.dev;
>> > struct drm_i915_private *dev_priv = to_i915(dev);
>> > - enum intel_display_power_domain power_domain;
>> >
>> > /*
>> > * See vlv_power_sequencer_reset() why we need
>> > * a power domain reference here.
>> > */
>> > - power_domain = intel_display_port_aux_power_domain(encoder);
>> > - intel_display_power_get(dev_priv, power_domain);
>> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>> >
>> > mutex_lock(&dev_priv->pps_mutex);
>> > }
>> > @@ -414,12 +412,10 @@ static void pps_unlock(struct intel_dp *intel_dp)
>> > struct intel_encoder *encoder = &intel_dig_port->base;
>> > struct drm_device *dev = encoder->base.dev;
>> > struct drm_i915_private *dev_priv = to_i915(dev);
>> > - enum intel_display_power_domain power_domain;
>> >
>> > mutex_unlock(&dev_priv->pps_mutex);
>> >
>> > - power_domain = intel_display_port_aux_power_domain(encoder);
>> > - intel_display_power_put(dev_priv, power_domain);
>> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
>> > }
>> >
>> > static void
>> > @@ -2005,9 +2001,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
>> > {
>> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> > - struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> > struct drm_i915_private *dev_priv = to_i915(dev);
>> > - enum intel_display_power_domain power_domain;
>> > u32 pp;
>> > i915_reg_t pp_stat_reg, pp_ctrl_reg;
>> > bool need_to_disable = !intel_dp->want_panel_vdd;
>> > @@ -2023,8 +2017,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
>> > if (edp_have_panel_vdd(intel_dp))
>> > return need_to_disable;
>> >
>> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
>> > - intel_display_power_get(dev_priv, power_domain);
>> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>> >
>> > DRM_DEBUG_KMS("Turning eDP port %c VDD on\n",
>> > port_name(intel_dig_port->port));
>> > @@ -2082,8 +2075,6 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
>> > struct drm_i915_private *dev_priv = to_i915(dev);
>> > struct intel_digital_port *intel_dig_port =
>> > dp_to_dig_port(intel_dp);
>> > - struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> > - enum intel_display_power_domain power_domain;
>> > u32 pp;
>> > i915_reg_t pp_stat_reg, pp_ctrl_reg;
>> >
>> > @@ -2113,8 +2104,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
>> > if ((pp & PANEL_POWER_ON) == 0)
>> > intel_dp->panel_power_off_time = ktime_get_boottime();
>> >
>> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
>> > - intel_display_power_put(dev_priv, power_domain);
>> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
>> > }
>> >
>> > static void edp_panel_vdd_work(struct work_struct *__work)
>> > @@ -2227,11 +2217,8 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
>> >
>> > static void edp_panel_off(struct intel_dp *intel_dp)
>> > {
>> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> > - struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> > struct drm_i915_private *dev_priv = to_i915(dev);
>> > - enum intel_display_power_domain power_domain;
>> > u32 pp;
>> > i915_reg_t pp_ctrl_reg;
>> >
>> > @@ -2263,8 +2250,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
>> > wait_panel_off(intel_dp);
>> >
>> > /* We got a reference when we enabled the VDD. */
>> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
>> > - intel_display_power_put(dev_priv, power_domain);
>> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
>> > }
>> >
>> > void intel_edp_panel_off(struct intel_dp *intel_dp)
>> > @@ -4587,11 +4573,9 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>> > struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> > struct drm_device *dev = connector->dev;
>> > enum drm_connector_status status;
>> > - enum intel_display_power_domain power_domain;
>> > u8 sink_irq_vector = 0;
>> >
>> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
>> > - intel_display_power_get(to_i915(dev), power_domain);
>> > + intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
>> >
>> > /* Can't disconnect eDP, but you can close the lid... */
>> > if (is_edp(intel_dp))
>> > @@ -4688,7 +4672,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>> > if (status != connector_status_connected && !intel_dp->is_mst)
>> > intel_dp_unset_edid(intel_dp);
>> >
>> > - intel_display_power_put(to_i915(dev), power_domain);
>> > + intel_display_power_put(to_i915(dev), intel_dp->aux_power_domain);
>> > return status;
>> > }
>> >
>> > @@ -4716,7 +4700,6 @@ intel_dp_force(struct drm_connector *connector)
>> > struct intel_dp *intel_dp = intel_attached_dp(connector);
>> > struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>> > struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
>> > - enum intel_display_power_domain power_domain;
>> >
>> > DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>> > connector->base.id, connector->name);
>> > @@ -4725,12 +4708,11 @@ intel_dp_force(struct drm_connector *connector)
>> > if (connector->status != connector_status_connected)
>> > return;
>> >
>> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
>> > - intel_display_power_get(dev_priv, power_domain);
>> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>> >
>> > intel_dp_set_edid(intel_dp);
>> >
>> > - intel_display_power_put(dev_priv, power_domain);
>> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
>> >
>> > if (intel_encoder->type != INTEL_OUTPUT_EDP)
>> > intel_encoder->type = INTEL_OUTPUT_DP;
>> > @@ -4965,7 +4947,6 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
>> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> > struct drm_device *dev = intel_dig_port->base.base.dev;
>> > struct drm_i915_private *dev_priv = to_i915(dev);
>> > - enum intel_display_power_domain power_domain;
>> >
>> > lockdep_assert_held(&dev_priv->pps_mutex);
>> >
>> > @@ -4979,8 +4960,7 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
>> > * indefinitely.
>> > */
>> > DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n");
>> > - power_domain = intel_display_port_aux_power_domain(&intel_dig_port->base);
>> > - intel_display_power_get(dev_priv, power_domain);
>> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>> >
>> > edp_panel_vdd_schedule_off(intel_dp);
>> > }
>> > @@ -5052,10 +5032,8 @@ enum irqreturn
>> > intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>> > {
>> > struct intel_dp *intel_dp = &intel_dig_port->dp;
>> > - struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> > struct drm_device *dev = intel_dig_port->base.base.dev;
>> > struct drm_i915_private *dev_priv = to_i915(dev);
>> > - enum intel_display_power_domain power_domain;
>> > enum irqreturn ret = IRQ_NONE;
>> >
>> > if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
>> > @@ -5083,8 +5061,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>> > return IRQ_NONE;
>> > }
>> >
>> > - power_domain = intel_display_port_aux_power_domain(intel_encoder);
>> > - intel_display_power_get(dev_priv, power_domain);
>> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>> >
>> > if (intel_dp->is_mst) {
>> > if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
>> > @@ -5112,7 +5089,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>> > ret = IRQ_HANDLED;
>> >
>> > put_power:
>> > - intel_display_power_put(dev_priv, power_domain);
>> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
>> >
>> > return ret;
>> > }
>> > @@ -5903,27 +5880,35 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> > return false;
>> > }
>> >
>> > +/* Set up the hotplug pin and aux power domain. */
>> > static void
>> > intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
>> > {
>> > struct intel_encoder *encoder = &intel_dig_port->base;
>> > + struct intel_dp *intel_dp = &intel_dig_port->dp;
>> >
>> > - /* Set up the hotplug pin. */
>> > switch (intel_dig_port->port) {
>> > case PORT_A:
>> > encoder->hpd_pin = HPD_PORT_A;
>> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A;
>> > break;
>> > case PORT_B:
>> > encoder->hpd_pin = HPD_PORT_B;
>> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B;
>> > break;
>> > case PORT_C:
>> > encoder->hpd_pin = HPD_PORT_C;
>> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C;
>> > break;
>> > case PORT_D:
>> > encoder->hpd_pin = HPD_PORT_D;
>> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
>> > break;
>> > case PORT_E:
>> > encoder->hpd_pin = HPD_PORT_E;
>> > +
>> > + /* FIXME: Check VBT for actual wiring of PORT E */
>> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
>> > break;
>> > default:
>> > MISSING_CASE(intel_dig_port->port);
>> > @@ -6003,6 +5988,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>> > connector->interlace_allowed = true;
>> > connector->doublescan_allowed = 0;
>> >
>> > + intel_dp_init_connector_port_info(intel_dig_port);
>> > +
>> > intel_dp_aux_init(intel_dp);
>> >
>> > INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
>> > @@ -6015,8 +6002,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>> > else
>> > intel_connector->get_hw_state = intel_connector_get_hw_state;
>> >
>> > - intel_dp_init_connector_port_info(intel_dig_port);
>> > -
>> > /* init MST on ports that can support it */
>> > if (HAS_DP_MST(dev_priv) && !is_edp(intel_dp) &&
>> > (port == PORT_B || port == PORT_C || port == PORT_D))
>> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > index 6e37fba..3ddeaa6 100644
>> > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > @@ -950,6 +950,7 @@ struct intel_dp {
>> > /* sink or branch descriptor */
>> > struct intel_dp_desc desc;
>> > struct drm_dp_aux aux;
>> > + enum intel_display_power_domain aux_power_domain;
>> > uint8_t train_set[4];
>> > int panel_power_up_delay;
>> > int panel_power_down_delay;
>> > @@ -1421,8 +1422,6 @@ void hsw_enable_ips(struct intel_crtc *crtc);
>> > void hsw_disable_ips(struct intel_crtc *crtc);
>> > enum intel_display_power_domain
>> > intel_display_port_power_domain(struct intel_encoder *intel_encoder);
>> > -enum intel_display_power_domain
>> > -intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder);
>> > void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>> > struct intel_crtc_state *pipe_config);
>> >
>> > --
>> > 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
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/6] drm/i915: Store encoder power domain in struct intel_encoder
2017-02-10 13:29 [PATCH 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
2017-02-10 13:29 ` [PATCH 1/6] drm/i915: Store aux power domain in intel_dp Ander Conselvan de Oliveira
@ 2017-02-10 13:29 ` Ander Conselvan de Oliveira
2017-02-16 9:50 ` Imre Deak
2017-02-10 13:29 ` [PATCH 3/6] drm/i915/glk: Implement WaDDIIOTimeout Ander Conselvan de Oliveira
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-10 13:29 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
The encoder power domain is obviously tied to the encoder, so store it
in struct intel_encoder. This avoids some indirection.
v2: Rebase
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_crt.c | 21 +++++++++------------
drivers/gpu/drm/i915/intel_ddi.c | 15 +++++++--------
drivers/gpu/drm/i915/intel_display.c | 31 ++-----------------------------
drivers/gpu/drm/i915/intel_dp.c | 8 ++++----
drivers/gpu/drm/i915/intel_dp_mst.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 4 ++--
drivers/gpu/drm/i915/intel_dsi.c | 9 +++++----
drivers/gpu/drm/i915/intel_dvo.c | 1 +
drivers/gpu/drm/i915/intel_hdmi.c | 8 ++++----
drivers/gpu/drm/i915/intel_lvds.c | 8 ++++----
drivers/gpu/drm/i915/intel_sdvo.c | 1 +
drivers/gpu/drm/i915/intel_tv.c | 1 +
12 files changed, 41 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 2bf5aca..8c82607 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -69,12 +69,11 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crt *crt = intel_encoder_to_crt(encoder);
- enum intel_display_power_domain power_domain;
u32 tmp;
bool ret;
- power_domain = intel_display_port_power_domain(encoder);
- if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+ if (!intel_display_power_get_if_enabled(dev_priv,
+ encoder->power_domain))
return false;
ret = false;
@@ -91,7 +90,7 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
ret = true;
out:
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, encoder->power_domain);
return ret;
}
@@ -676,7 +675,6 @@ intel_crt_detect(struct drm_connector *connector, bool force)
struct drm_i915_private *dev_priv = to_i915(connector->dev);
struct intel_crt *crt = intel_attached_crt(connector);
struct intel_encoder *intel_encoder = &crt->base;
- enum intel_display_power_domain power_domain;
enum drm_connector_status status;
struct intel_load_detect_pipe tmp;
struct drm_modeset_acquire_ctx ctx;
@@ -689,8 +687,7 @@ intel_crt_detect(struct drm_connector *connector, bool force)
if (dmi_check_system(intel_spurious_crt_detect))
return connector_status_disconnected;
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
+ intel_display_power_get(dev_priv, intel_encoder->power_domain);
if (I915_HAS_HOTPLUG(dev_priv)) {
/* We can not rely on the HPD pin always being correctly wired
@@ -745,7 +742,7 @@ intel_crt_detect(struct drm_connector *connector, bool force)
drm_modeset_acquire_fini(&ctx);
out:
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, intel_encoder->power_domain);
return status;
}
@@ -761,12 +758,10 @@ static int intel_crt_get_modes(struct drm_connector *connector)
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crt *crt = intel_attached_crt(connector);
struct intel_encoder *intel_encoder = &crt->base;
- enum intel_display_power_domain power_domain;
int ret;
struct i2c_adapter *i2c;
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
+ intel_display_power_get(dev_priv, intel_encoder->power_domain);
i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin);
ret = intel_crt_ddc_get_modes(connector, i2c);
@@ -778,7 +773,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
ret = intel_crt_ddc_get_modes(connector, i2c);
out:
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, intel_encoder->power_domain);
return ret;
}
@@ -904,6 +899,8 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
crt->adpa_reg = adpa_reg;
+ crt->base.power_domain = POWER_DOMAIN_PORT_CRT;
+
crt->base.compute_config = intel_crt_compute_config;
if (HAS_PCH_SPLIT(dev_priv)) {
crt->base.disable = pch_disable_crt;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cd6fedd..b0c4d23 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1316,12 +1316,11 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
enum port port = intel_ddi_get_encoder_port(intel_encoder);
enum pipe pipe = 0;
enum transcoder cpu_transcoder;
- enum intel_display_power_domain power_domain;
uint32_t tmp;
bool ret;
- power_domain = intel_display_port_power_domain(intel_encoder);
- if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+ if (!intel_display_power_get_if_enabled(dev_priv,
+ intel_encoder->power_domain))
return false;
if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
@@ -1363,7 +1362,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
}
out:
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, intel_encoder->power_domain);
return ret;
}
@@ -1374,13 +1373,12 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
enum port port = intel_ddi_get_encoder_port(encoder);
- enum intel_display_power_domain power_domain;
u32 tmp;
int i;
bool ret;
- power_domain = intel_display_port_power_domain(encoder);
- if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+ if (!intel_display_power_get_if_enabled(dev_priv,
+ encoder->power_domain))
return false;
ret = false;
@@ -1437,7 +1435,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
"(PHY_CTL %08x)\n", port_name(port), tmp);
}
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, encoder->power_domain);
return ret;
}
@@ -2216,6 +2214,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
intel_dig_port->max_lanes = max_lanes;
intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
+ intel_encoder->power_domain = intel_port_to_power_domain(port);
intel_encoder->port = port;
intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
intel_encoder->cloneable = 0;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1e70776..585d149 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5570,7 +5570,7 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
I915_WRITE(BCLRPAT(crtc->pipe), 0);
}
-static enum intel_display_power_domain port_to_power_domain(enum port port)
+enum intel_display_power_domain intel_port_to_power_domain(enum port port)
{
switch (port) {
case PORT_A:
@@ -5589,33 +5589,6 @@ static enum intel_display_power_domain port_to_power_domain(enum port port)
}
}
-enum intel_display_power_domain
-intel_display_port_power_domain(struct intel_encoder *intel_encoder)
-{
- struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
- struct intel_digital_port *intel_dig_port;
-
- switch (intel_encoder->type) {
- case INTEL_OUTPUT_UNKNOWN:
- /* Only DDI platforms should ever use this output type */
- WARN_ON_ONCE(!HAS_DDI(dev_priv));
- case INTEL_OUTPUT_DP:
- case INTEL_OUTPUT_HDMI:
- case INTEL_OUTPUT_EDP:
- intel_dig_port = enc_to_dig_port(&intel_encoder->base);
- return port_to_power_domain(intel_dig_port->port);
- case INTEL_OUTPUT_DP_MST:
- intel_dig_port = enc_to_mst(&intel_encoder->base)->primary;
- return port_to_power_domain(intel_dig_port->port);
- case INTEL_OUTPUT_ANALOG:
- return POWER_DOMAIN_PORT_CRT;
- case INTEL_OUTPUT_DSI:
- return POWER_DOMAIN_PORT_DSI;
- default:
- return POWER_DOMAIN_PORT_OTHER;
- }
-}
-
static u64 get_crtc_power_domains(struct drm_crtc *crtc,
struct intel_crtc_state *crtc_state)
{
@@ -5639,7 +5612,7 @@ static u64 get_crtc_power_domains(struct drm_crtc *crtc,
drm_for_each_encoder_mask(encoder, dev, crtc_state->base.encoder_mask) {
struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
- mask |= BIT_ULL(intel_display_port_power_domain(intel_encoder));
+ mask |= BIT_ULL(intel_encoder->power_domain);
}
if (HAS_DDI(dev_priv) && crtc_state->has_audio)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e2e4766..2e605ad 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2496,12 +2496,11 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
enum port port = dp_to_dig_port(intel_dp)->port;
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- enum intel_display_power_domain power_domain;
u32 tmp;
bool ret;
- power_domain = intel_display_port_power_domain(encoder);
- if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+ if (!intel_display_power_get_if_enabled(dev_priv,
+ encoder->power_domain))
return false;
ret = false;
@@ -2537,7 +2536,7 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
ret = true;
out:
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, encoder->power_domain);
return ret;
}
@@ -6086,6 +6085,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
intel_dig_port->max_lanes = 4;
intel_encoder->type = INTEL_OUTPUT_DP;
+ intel_encoder->power_domain = intel_port_to_power_domain(port);
if (IS_CHERRYVIEW(dev_priv)) {
if (port == PORT_D)
intel_encoder->crtc_mask = 1 << 2;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 6a85d38..d94fd4b 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -548,6 +548,7 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
DRM_MODE_ENCODER_DPMST, "DP-MST %c", pipe_name(pipe));
intel_encoder->type = INTEL_OUTPUT_DP_MST;
+ intel_encoder->power_domain = intel_dig_port->base.power_domain;
intel_encoder->port = intel_dig_port->port;
intel_encoder->crtc_mask = 0x7;
intel_encoder->cloneable = 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3ddeaa6..c8873c0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -249,6 +249,7 @@ struct intel_encoder {
void (*suspend)(struct intel_encoder *);
int crtc_mask;
enum hpd_pin hpd_pin;
+ enum intel_display_power_domain power_domain;
/* for communication with audio component; protected by av_mutex */
const struct drm_connector *audio_connector;
};
@@ -1420,8 +1421,7 @@ int chv_calc_dpll_params(int refclk, struct dpll *pll_clock);
bool intel_crtc_active(struct intel_crtc *crtc);
void hsw_enable_ips(struct intel_crtc *crtc);
void hsw_disable_ips(struct intel_crtc *crtc);
-enum intel_display_power_domain
-intel_display_port_power_domain(struct intel_encoder *intel_encoder);
+enum intel_display_power_domain intel_port_to_power_domain(enum port port);
void intel_mode_from_pipe_config(struct drm_display_mode *mode,
struct intel_crtc_state *pipe_config);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index c98234e..517a943 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -768,14 +768,13 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
- enum intel_display_power_domain power_domain;
enum port port;
bool active = false;
DRM_DEBUG_KMS("\n");
- power_domain = intel_display_port_power_domain(encoder);
- if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+ if (!intel_display_power_get_if_enabled(dev_priv,
+ encoder->power_domain))
return false;
/*
@@ -831,7 +830,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
}
out_put_power:
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, encoder->power_domain);
return active;
}
@@ -1509,6 +1508,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
intel_connector->get_hw_state = intel_connector_get_hw_state;
intel_encoder->port = port;
+
/*
* On BYT/CHV, pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI
* port C. BXT isn't limited like this.
@@ -1595,6 +1595,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
}
intel_encoder->type = INTEL_OUTPUT_DSI;
+ intel_encoder->power_domain = POWER_DOMAIN_PORT_DSI;
intel_encoder->cloneable = 0;
drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
DRM_MODE_CONNECTOR_DSI);
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 50da89d..6025839 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -515,6 +515,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
"DVO %c", port_name(port));
intel_encoder->type = INTEL_OUTPUT_DVO;
+ intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
intel_encoder->port = port;
intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index a580de8..c2184f7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -902,12 +902,11 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
- enum intel_display_power_domain power_domain;
u32 tmp;
bool ret;
- power_domain = intel_display_port_power_domain(encoder);
- if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+ if (!intel_display_power_get_if_enabled(dev_priv,
+ encoder->power_domain))
return false;
ret = false;
@@ -927,7 +926,7 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
ret = true;
out:
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, encoder->power_domain);
return ret;
}
@@ -1981,6 +1980,7 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
}
intel_encoder->type = INTEL_OUTPUT_HDMI;
+ intel_encoder->power_domain = intel_port_to_power_domain(port);
intel_encoder->port = port;
if (IS_CHERRYVIEW(dev_priv)) {
if (port == PORT_D)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 9ca4dc4..8b942ef 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -91,12 +91,11 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
- enum intel_display_power_domain power_domain;
u32 tmp;
bool ret;
- power_domain = intel_display_port_power_domain(encoder);
- if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+ if (!intel_display_power_get_if_enabled(dev_priv,
+ encoder->power_domain))
return false;
ret = false;
@@ -114,7 +113,7 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
ret = true;
out:
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, encoder->power_domain);
return ret;
}
@@ -1066,6 +1065,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
intel_connector_attach_encoder(intel_connector, intel_encoder);
intel_encoder->type = INTEL_OUTPUT_LVDS;
+ intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
intel_encoder->port = PORT_NONE;
intel_encoder->cloneable = 0;
if (HAS_PCH_SPLIT(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 2ad1390..816a6f5 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2981,6 +2981,7 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
/* encoder type will be decided later */
intel_encoder = &intel_sdvo->base;
intel_encoder->type = INTEL_OUTPUT_SDVO;
+ intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
intel_encoder->port = port;
drm_encoder_init(&dev_priv->drm, &intel_encoder->base,
&intel_sdvo_enc_funcs, 0,
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index eb692e4..6ed1a3c 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1621,6 +1621,7 @@ intel_tv_init(struct drm_i915_private *dev_priv)
intel_connector_attach_encoder(intel_connector, intel_encoder);
intel_encoder->type = INTEL_OUTPUT_TVOUT;
+ intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
intel_encoder->port = PORT_NONE;
intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
intel_encoder->cloneable = 0;
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/6] drm/i915: Store encoder power domain in struct intel_encoder
2017-02-10 13:29 ` [PATCH 2/6] drm/i915: Store encoder power domain in struct intel_encoder Ander Conselvan de Oliveira
@ 2017-02-16 9:50 ` Imre Deak
0 siblings, 0 replies; 17+ messages in thread
From: Imre Deak @ 2017-02-16 9:50 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
On Fri, Feb 10, 2017 at 03:29:55PM +0200, Ander Conselvan de Oliveira wrote:
> The encoder power domain is obviously tied to the encoder, so store it
> in struct intel_encoder. This avoids some indirection.
>
> v2: Rebase
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/intel_crt.c | 21 +++++++++------------
> drivers/gpu/drm/i915/intel_ddi.c | 15 +++++++--------
> drivers/gpu/drm/i915/intel_display.c | 31 ++-----------------------------
> drivers/gpu/drm/i915/intel_dp.c | 8 ++++----
> drivers/gpu/drm/i915/intel_dp_mst.c | 1 +
> drivers/gpu/drm/i915/intel_drv.h | 4 ++--
> drivers/gpu/drm/i915/intel_dsi.c | 9 +++++----
> drivers/gpu/drm/i915/intel_dvo.c | 1 +
> drivers/gpu/drm/i915/intel_hdmi.c | 8 ++++----
> drivers/gpu/drm/i915/intel_lvds.c | 8 ++++----
> drivers/gpu/drm/i915/intel_sdvo.c | 1 +
> drivers/gpu/drm/i915/intel_tv.c | 1 +
> 12 files changed, 41 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 2bf5aca..8c82607 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -69,12 +69,11 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
> struct drm_device *dev = encoder->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crt *crt = intel_encoder_to_crt(encoder);
> - enum intel_display_power_domain power_domain;
> u32 tmp;
> bool ret;
>
> - power_domain = intel_display_port_power_domain(encoder);
> - if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> + if (!intel_display_power_get_if_enabled(dev_priv,
> + encoder->power_domain))
> return false;
>
> ret = false;
> @@ -91,7 +90,7 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
>
> ret = true;
> out:
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, encoder->power_domain);
>
> return ret;
> }
> @@ -676,7 +675,6 @@ intel_crt_detect(struct drm_connector *connector, bool force)
> struct drm_i915_private *dev_priv = to_i915(connector->dev);
> struct intel_crt *crt = intel_attached_crt(connector);
> struct intel_encoder *intel_encoder = &crt->base;
> - enum intel_display_power_domain power_domain;
> enum drm_connector_status status;
> struct intel_load_detect_pipe tmp;
> struct drm_modeset_acquire_ctx ctx;
> @@ -689,8 +687,7 @@ intel_crt_detect(struct drm_connector *connector, bool force)
> if (dmi_check_system(intel_spurious_crt_detect))
> return connector_status_disconnected;
>
> - power_domain = intel_display_port_power_domain(intel_encoder);
> - intel_display_power_get(dev_priv, power_domain);
> + intel_display_power_get(dev_priv, intel_encoder->power_domain);
>
> if (I915_HAS_HOTPLUG(dev_priv)) {
> /* We can not rely on the HPD pin always being correctly wired
> @@ -745,7 +742,7 @@ intel_crt_detect(struct drm_connector *connector, bool force)
> drm_modeset_acquire_fini(&ctx);
>
> out:
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, intel_encoder->power_domain);
> return status;
> }
>
> @@ -761,12 +758,10 @@ static int intel_crt_get_modes(struct drm_connector *connector)
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crt *crt = intel_attached_crt(connector);
> struct intel_encoder *intel_encoder = &crt->base;
> - enum intel_display_power_domain power_domain;
> int ret;
> struct i2c_adapter *i2c;
>
> - power_domain = intel_display_port_power_domain(intel_encoder);
> - intel_display_power_get(dev_priv, power_domain);
> + intel_display_power_get(dev_priv, intel_encoder->power_domain);
>
> i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin);
> ret = intel_crt_ddc_get_modes(connector, i2c);
> @@ -778,7 +773,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
> ret = intel_crt_ddc_get_modes(connector, i2c);
>
> out:
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, intel_encoder->power_domain);
>
> return ret;
> }
> @@ -904,6 +899,8 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
>
> crt->adpa_reg = adpa_reg;
>
> + crt->base.power_domain = POWER_DOMAIN_PORT_CRT;
> +
> crt->base.compute_config = intel_crt_compute_config;
> if (HAS_PCH_SPLIT(dev_priv)) {
> crt->base.disable = pch_disable_crt;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cd6fedd..b0c4d23 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1316,12 +1316,11 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
> enum port port = intel_ddi_get_encoder_port(intel_encoder);
> enum pipe pipe = 0;
> enum transcoder cpu_transcoder;
> - enum intel_display_power_domain power_domain;
> uint32_t tmp;
> bool ret;
>
> - power_domain = intel_display_port_power_domain(intel_encoder);
> - if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> + if (!intel_display_power_get_if_enabled(dev_priv,
> + intel_encoder->power_domain))
> return false;
>
> if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> @@ -1363,7 +1362,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
> }
>
> out:
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, intel_encoder->power_domain);
>
> return ret;
> }
> @@ -1374,13 +1373,12 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> struct drm_device *dev = encoder->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> enum port port = intel_ddi_get_encoder_port(encoder);
> - enum intel_display_power_domain power_domain;
> u32 tmp;
> int i;
> bool ret;
>
> - power_domain = intel_display_port_power_domain(encoder);
> - if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> + if (!intel_display_power_get_if_enabled(dev_priv,
> + encoder->power_domain))
> return false;
>
> ret = false;
> @@ -1437,7 +1435,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> "(PHY_CTL %08x)\n", port_name(port), tmp);
> }
>
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, encoder->power_domain);
>
> return ret;
> }
> @@ -2216,6 +2214,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> intel_dig_port->max_lanes = max_lanes;
>
> intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
> + intel_encoder->power_domain = intel_port_to_power_domain(port);
> intel_encoder->port = port;
> intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> intel_encoder->cloneable = 0;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1e70776..585d149 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5570,7 +5570,7 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
> I915_WRITE(BCLRPAT(crtc->pipe), 0);
> }
>
> -static enum intel_display_power_domain port_to_power_domain(enum port port)
> +enum intel_display_power_domain intel_port_to_power_domain(enum port port)
> {
> switch (port) {
> case PORT_A:
> @@ -5589,33 +5589,6 @@ static enum intel_display_power_domain port_to_power_domain(enum port port)
> }
> }
>
> -enum intel_display_power_domain
> -intel_display_port_power_domain(struct intel_encoder *intel_encoder)
> -{
> - struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
> - struct intel_digital_port *intel_dig_port;
> -
> - switch (intel_encoder->type) {
> - case INTEL_OUTPUT_UNKNOWN:
> - /* Only DDI platforms should ever use this output type */
> - WARN_ON_ONCE(!HAS_DDI(dev_priv));
> - case INTEL_OUTPUT_DP:
> - case INTEL_OUTPUT_HDMI:
> - case INTEL_OUTPUT_EDP:
> - intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> - return port_to_power_domain(intel_dig_port->port);
> - case INTEL_OUTPUT_DP_MST:
> - intel_dig_port = enc_to_mst(&intel_encoder->base)->primary;
> - return port_to_power_domain(intel_dig_port->port);
> - case INTEL_OUTPUT_ANALOG:
> - return POWER_DOMAIN_PORT_CRT;
> - case INTEL_OUTPUT_DSI:
> - return POWER_DOMAIN_PORT_DSI;
> - default:
> - return POWER_DOMAIN_PORT_OTHER;
> - }
> -}
> -
> static u64 get_crtc_power_domains(struct drm_crtc *crtc,
> struct intel_crtc_state *crtc_state)
> {
> @@ -5639,7 +5612,7 @@ static u64 get_crtc_power_domains(struct drm_crtc *crtc,
> drm_for_each_encoder_mask(encoder, dev, crtc_state->base.encoder_mask) {
> struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
>
> - mask |= BIT_ULL(intel_display_port_power_domain(intel_encoder));
> + mask |= BIT_ULL(intel_encoder->power_domain);
> }
>
> if (HAS_DDI(dev_priv) && crtc_state->has_audio)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e2e4766..2e605ad 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2496,12 +2496,11 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
> enum port port = dp_to_dig_port(intel_dp)->port;
> struct drm_device *dev = encoder->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - enum intel_display_power_domain power_domain;
> u32 tmp;
> bool ret;
>
> - power_domain = intel_display_port_power_domain(encoder);
> - if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> + if (!intel_display_power_get_if_enabled(dev_priv,
> + encoder->power_domain))
> return false;
>
> ret = false;
> @@ -2537,7 +2536,7 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
> ret = true;
>
> out:
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, encoder->power_domain);
>
> return ret;
> }
> @@ -6086,6 +6085,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> intel_dig_port->max_lanes = 4;
>
> intel_encoder->type = INTEL_OUTPUT_DP;
> + intel_encoder->power_domain = intel_port_to_power_domain(port);
> if (IS_CHERRYVIEW(dev_priv)) {
> if (port == PORT_D)
> intel_encoder->crtc_mask = 1 << 2;
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 6a85d38..d94fd4b 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -548,6 +548,7 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
> DRM_MODE_ENCODER_DPMST, "DP-MST %c", pipe_name(pipe));
>
> intel_encoder->type = INTEL_OUTPUT_DP_MST;
> + intel_encoder->power_domain = intel_dig_port->base.power_domain;
> intel_encoder->port = intel_dig_port->port;
> intel_encoder->crtc_mask = 0x7;
> intel_encoder->cloneable = 0;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3ddeaa6..c8873c0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -249,6 +249,7 @@ struct intel_encoder {
> void (*suspend)(struct intel_encoder *);
> int crtc_mask;
> enum hpd_pin hpd_pin;
> + enum intel_display_power_domain power_domain;
> /* for communication with audio component; protected by av_mutex */
> const struct drm_connector *audio_connector;
> };
> @@ -1420,8 +1421,7 @@ int chv_calc_dpll_params(int refclk, struct dpll *pll_clock);
> bool intel_crtc_active(struct intel_crtc *crtc);
> void hsw_enable_ips(struct intel_crtc *crtc);
> void hsw_disable_ips(struct intel_crtc *crtc);
> -enum intel_display_power_domain
> -intel_display_port_power_domain(struct intel_encoder *intel_encoder);
> +enum intel_display_power_domain intel_port_to_power_domain(enum port port);
> void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> struct intel_crtc_state *pipe_config);
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index c98234e..517a943 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -768,14 +768,13 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> - enum intel_display_power_domain power_domain;
> enum port port;
> bool active = false;
>
> DRM_DEBUG_KMS("\n");
>
> - power_domain = intel_display_port_power_domain(encoder);
> - if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> + if (!intel_display_power_get_if_enabled(dev_priv,
> + encoder->power_domain))
> return false;
>
> /*
> @@ -831,7 +830,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> }
>
> out_put_power:
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, encoder->power_domain);
>
> return active;
> }
> @@ -1509,6 +1508,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
> intel_connector->get_hw_state = intel_connector_get_hw_state;
>
> intel_encoder->port = port;
> +
> /*
> * On BYT/CHV, pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI
> * port C. BXT isn't limited like this.
> @@ -1595,6 +1595,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
> }
>
> intel_encoder->type = INTEL_OUTPUT_DSI;
> + intel_encoder->power_domain = POWER_DOMAIN_PORT_DSI;
> intel_encoder->cloneable = 0;
> drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
> DRM_MODE_CONNECTOR_DSI);
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 50da89d..6025839 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -515,6 +515,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
> "DVO %c", port_name(port));
>
> intel_encoder->type = INTEL_OUTPUT_DVO;
> + intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
> intel_encoder->port = port;
> intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index a580de8..c2184f7 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -902,12 +902,11 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
> struct drm_device *dev = encoder->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> - enum intel_display_power_domain power_domain;
> u32 tmp;
> bool ret;
>
> - power_domain = intel_display_port_power_domain(encoder);
> - if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> + if (!intel_display_power_get_if_enabled(dev_priv,
> + encoder->power_domain))
> return false;
>
> ret = false;
> @@ -927,7 +926,7 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
> ret = true;
>
> out:
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, encoder->power_domain);
>
> return ret;
> }
> @@ -1981,6 +1980,7 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
> }
>
> intel_encoder->type = INTEL_OUTPUT_HDMI;
> + intel_encoder->power_domain = intel_port_to_power_domain(port);
> intel_encoder->port = port;
> if (IS_CHERRYVIEW(dev_priv)) {
> if (port == PORT_D)
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 9ca4dc4..8b942ef 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -91,12 +91,11 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
> struct drm_device *dev = encoder->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
> - enum intel_display_power_domain power_domain;
> u32 tmp;
> bool ret;
>
> - power_domain = intel_display_port_power_domain(encoder);
> - if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> + if (!intel_display_power_get_if_enabled(dev_priv,
> + encoder->power_domain))
> return false;
>
> ret = false;
> @@ -114,7 +113,7 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
> ret = true;
>
> out:
> - intel_display_power_put(dev_priv, power_domain);
> + intel_display_power_put(dev_priv, encoder->power_domain);
>
> return ret;
> }
> @@ -1066,6 +1065,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> intel_connector_attach_encoder(intel_connector, intel_encoder);
>
> intel_encoder->type = INTEL_OUTPUT_LVDS;
> + intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
> intel_encoder->port = PORT_NONE;
> intel_encoder->cloneable = 0;
> if (HAS_PCH_SPLIT(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 2ad1390..816a6f5 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2981,6 +2981,7 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
> /* encoder type will be decided later */
> intel_encoder = &intel_sdvo->base;
> intel_encoder->type = INTEL_OUTPUT_SDVO;
> + intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
> intel_encoder->port = port;
> drm_encoder_init(&dev_priv->drm, &intel_encoder->base,
> &intel_sdvo_enc_funcs, 0,
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index eb692e4..6ed1a3c 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1621,6 +1621,7 @@ intel_tv_init(struct drm_i915_private *dev_priv)
> intel_connector_attach_encoder(intel_connector, intel_encoder);
>
> intel_encoder->type = INTEL_OUTPUT_TVOUT;
> + intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
> intel_encoder->port = PORT_NONE;
> intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
> intel_encoder->cloneable = 0;
> --
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/6] drm/i915/glk: Implement WaDDIIOTimeout
2017-02-10 13:29 [PATCH 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
2017-02-10 13:29 ` [PATCH 1/6] drm/i915: Store aux power domain in intel_dp Ander Conselvan de Oliveira
2017-02-10 13:29 ` [PATCH 2/6] drm/i915: Store encoder power domain in struct intel_encoder Ander Conselvan de Oliveira
@ 2017-02-10 13:29 ` Ander Conselvan de Oliveira
2017-02-16 10:07 ` Imre Deak
2017-02-10 13:29 ` [PATCH 4/6] drm/i915/glk: Don't enable DDI IO power domains during init Ander Conselvan de Oliveira
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-10 13:29 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
Implement WaDDIIOTimeout to avoid a timeout when enabling the DDI IO
power domains.
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 6 ++++++
drivers/gpu/drm/i915/i915_reg.h | 5 +++++
drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++++
3 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ceb7699..bfccf9d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2774,6 +2774,12 @@ intel_info(const struct drm_i915_private *dev_priv)
#define IS_KBL_REVID(dev_priv, since, until) \
(IS_KABYLAKE(dev_priv) && IS_REVID(dev_priv, since, until))
+#define GLK_REVID_A0 0x0
+#define GLK_REVID_A1 0x1
+
+#define IS_GLK_REVID(dev_priv, since, until) \
+ (IS_GEMINILAKE(dev_priv) && IS_REVID(dev_priv, since, until))
+
/*
* The genX designation typically refers to the render engine, so render
* capability related checks should use IS_GEN, while display and other checks
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 141a5c1..ce6f096 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6495,6 +6495,11 @@ enum {
#define CHICKEN_PAR2_1 _MMIO(0x42090)
#define KVM_CONFIG_CHANGE_NOTIFICATION_SELECT (1 << 14)
+#define CHICKEN_MISC_2 _MMIO(0x42084)
+#define GLK_CL0_PWR_DOWN (1 << 10)
+#define GLK_CL1_PWR_DOWN (1 << 11)
+#define GLK_CL2_PWR_DOWN (1 << 12)
+
#define _CHICKEN_PIPESL_1_A 0x420b0
#define _CHICKEN_PIPESL_1_B 0x420b4
#define HSW_FBCQ_DIS (1 << 22)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c0b0f5a..6f2a95f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -115,6 +115,16 @@ static void glk_init_clock_gating(struct drm_i915_private *dev_priv)
*/
I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
PWM1_GATING_DIS | PWM2_GATING_DIS);
+
+ /* WaDDIIOTimeout:glk */
+ if (IS_GLK_REVID(dev_priv, 0, GLK_REVID_A1)) {
+ u32 val = I915_READ(CHICKEN_MISC_2);
+ val &= ~(GLK_CL0_PWR_DOWN |
+ GLK_CL1_PWR_DOWN |
+ GLK_CL2_PWR_DOWN);
+ I915_WRITE(CHICKEN_MISC_2, val);
+ }
+
}
static void i915_pineview_get_mem_freq(struct drm_i915_private *dev_priv)
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/6] drm/i915/glk: Implement WaDDIIOTimeout
2017-02-10 13:29 ` [PATCH 3/6] drm/i915/glk: Implement WaDDIIOTimeout Ander Conselvan de Oliveira
@ 2017-02-16 10:07 ` Imre Deak
0 siblings, 0 replies; 17+ messages in thread
From: Imre Deak @ 2017-02-16 10:07 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
On Fri, Feb 10, 2017 at 03:29:56PM +0200, Ander Conselvan de Oliveira wrote:
> Implement WaDDIIOTimeout to avoid a timeout when enabling the DDI IO
> power domains.
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 ++++++
> drivers/gpu/drm/i915/i915_reg.h | 5 +++++
> drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++++
> 3 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ceb7699..bfccf9d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2774,6 +2774,12 @@ intel_info(const struct drm_i915_private *dev_priv)
> #define IS_KBL_REVID(dev_priv, since, until) \
> (IS_KABYLAKE(dev_priv) && IS_REVID(dev_priv, since, until))
>
> +#define GLK_REVID_A0 0x0
> +#define GLK_REVID_A1 0x1
> +
> +#define IS_GLK_REVID(dev_priv, since, until) \
> + (IS_GEMINILAKE(dev_priv) && IS_REVID(dev_priv, since, until))
> +
> /*
> * The genX designation typically refers to the render engine, so render
> * capability related checks should use IS_GEN, while display and other checks
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 141a5c1..ce6f096 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6495,6 +6495,11 @@ enum {
> #define CHICKEN_PAR2_1 _MMIO(0x42090)
> #define KVM_CONFIG_CHANGE_NOTIFICATION_SELECT (1 << 14)
>
> +#define CHICKEN_MISC_2 _MMIO(0x42084)
> +#define GLK_CL0_PWR_DOWN (1 << 10)
> +#define GLK_CL1_PWR_DOWN (1 << 11)
> +#define GLK_CL2_PWR_DOWN (1 << 12)
> +
> #define _CHICKEN_PIPESL_1_A 0x420b0
> #define _CHICKEN_PIPESL_1_B 0x420b4
> #define HSW_FBCQ_DIS (1 << 22)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c0b0f5a..6f2a95f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -115,6 +115,16 @@ static void glk_init_clock_gating(struct drm_i915_private *dev_priv)
> */
> I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
> PWM1_GATING_DIS | PWM2_GATING_DIS);
> +
> + /* WaDDIIOTimeout:glk */
> + if (IS_GLK_REVID(dev_priv, 0, GLK_REVID_A1)) {
> + u32 val = I915_READ(CHICKEN_MISC_2);
> + val &= ~(GLK_CL0_PWR_DOWN |
> + GLK_CL1_PWR_DOWN |
> + GLK_CL2_PWR_DOWN);
> + I915_WRITE(CHICKEN_MISC_2, val);
> + }
> +
> }
>
> static void i915_pineview_get_mem_freq(struct drm_i915_private *dev_priv)
> --
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/6] drm/i915/glk: Don't enable DDI IO power domains during init
2017-02-10 13:29 [PATCH 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
` (2 preceding siblings ...)
2017-02-10 13:29 ` [PATCH 3/6] drm/i915/glk: Implement WaDDIIOTimeout Ander Conselvan de Oliveira
@ 2017-02-10 13:29 ` Ander Conselvan de Oliveira
2017-02-13 16:31 ` David Weinehall
2017-02-10 13:29 ` [PATCH 5/6] drm/i915/glk: Don't attempt to sync DDI IO power well hw state Ander Conselvan de Oliveira
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-10 13:29 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
In Geminilake, the DDI IO power domains can't be enabled before a DPLL
is running and mapped to the appropriate DDI. At least on Geminilake,
attempting to enable those during init will lead to a timeout.
The failure to enable the power domain also causes issues with the state
verifier during resume from suspend. After all the init power domains
are enabled, the call to intel_power_domains_sync_hw() from the resume
path will cause the hw_enabled field on the respective power wells to be
false while the usage count remains above zero. Further attempts to
enable the power domain caused by a modeset will simply update the usage
count without doing anything else. When the state verifier attempts to
read the state of a DDI encoder, intel_display_power_get_if_enabled()
returns false, leading to the following WARN:
WARNING: CPU: 3 PID: 1743 at drivers/gpu/drm/i915/intel_display.c:7001 verify_connector_state.isra.80+0x26c/0x2b0 [i915]
attached crtc is active, but connector isn't
Modules linked in: i915(E) tun ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_broute bridge stp llc ebtable_nat ip6table_mangle ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_raw iptable_mangle iptable_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables x86_pkg_temp_thermal coretemp kvm_intel kvm i2c_algo_bit drm_kms_helper irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel drm shpchp tpm_tis tpm_tis_core tpm nfsd auth_rpcgss nfs_acl lockd grace sunrpc crc32c_intel serio_raw [last unloaded: i915]
CPU: 3 PID: 1743 Comm: kworker/u8:22 Tainted: G W E 4.10.0-rc3ander+ #300
Hardware name: Intel Corp. Geminilake/GLK RVP1 DDR4 (05), BIOS GELKRVPA.X64.0023.B40.1611302145 11/30/2016
Workqueue: events_unbound async_run_entry_fn
Call Trace:
dump_stack+0x86/0xc3
__warn+0xcb/0xf0
warn_slowpath_fmt+0x5f/0x80
verify_connector_state.isra.80+0x26c/0x2b0 [i915]
intel_atomic_commit_tail+0x520/0x1000 [i915]
? remove_wait_queue+0x70/0x70
intel_atomic_commit+0x3f8/0x520 [i915]
? intel_runtime_pm_put+0x6e/0xa0 [i915]
drm_atomic_commit+0x4b/0x50 [drm]
__intel_display_resume+0x72/0xc0 [i915]
intel_display_resume+0x107/0x150 [i915]
i915_drm_resume+0xe0/0x180 [i915]
i915_pm_restore+0x1e/0x30 [i915]
i915_pm_resume+0xe/0x10 [i915]
pci_pm_resume+0x64/0xa0
dpm_run_callback+0xa1/0x2a0
? pci_pm_thaw+0x90/0x90
device_resume+0xe3/0x200
async_resume+0x1d/0x50
async_run_entry_fn+0x39/0x170
process_one_work+0x212/0x670
? process_one_work+0x197/0x670
worker_thread+0x4e/0x490
kthread+0x101/0x140
? process_one_work+0x670/0x670
? kthread_create_on_node+0x60/0x60
ret_from_fork+0x2a/0x40
Cc: David Weinehall <david.weinehall@linux.intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8795679..b729a39 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -469,14 +469,11 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
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) | \
- BIT_ULL(POWER_DOMAIN_INIT))
+ BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES))
#define GLK_DISPLAY_DDI_B_POWER_DOMAINS ( \
- BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) | \
- BIT_ULL(POWER_DOMAIN_INIT))
+ BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES))
#define GLK_DISPLAY_DDI_C_POWER_DOMAINS ( \
- BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES) | \
- BIT_ULL(POWER_DOMAIN_INIT))
+ BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES))
#define GLK_DPIO_CMN_A_POWER_DOMAINS ( \
BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) | \
BIT_ULL(POWER_DOMAIN_AUX_A) | \
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/6] drm/i915/glk: Don't enable DDI IO power domains during init
2017-02-10 13:29 ` [PATCH 4/6] drm/i915/glk: Don't enable DDI IO power domains during init Ander Conselvan de Oliveira
@ 2017-02-13 16:31 ` David Weinehall
0 siblings, 0 replies; 17+ messages in thread
From: David Weinehall @ 2017-02-13 16:31 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
On Fri, Feb 10, 2017 at 03:29:57PM +0200, Ander Conselvan de Oliveira wrote:
> In Geminilake, the DDI IO power domains can't be enabled before a DPLL
> is running and mapped to the appropriate DDI. At least on Geminilake,
> attempting to enable those during init will lead to a timeout.
>
> The failure to enable the power domain also causes issues with the state
> verifier during resume from suspend. After all the init power domains
> are enabled, the call to intel_power_domains_sync_hw() from the resume
> path will cause the hw_enabled field on the respective power wells to be
> false while the usage count remains above zero. Further attempts to
> enable the power domain caused by a modeset will simply update the usage
> count without doing anything else. When the state verifier attempts to
> read the state of a DDI encoder, intel_display_power_get_if_enabled()
> returns false, leading to the following WARN:
>
> WARNING: CPU: 3 PID: 1743 at drivers/gpu/drm/i915/intel_display.c:7001 verify_connector_state.isra.80+0x26c/0x2b0 [i915]
> attached crtc is active, but connector isn't
> Modules linked in: i915(E) tun ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_broute bridge stp llc ebtable_nat ip6table_mangle ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_raw iptable_mangle iptable_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables x86_pkg_temp_thermal coretemp kvm_intel kvm i2c_algo_bit drm_kms_helper irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel drm shpchp tpm_tis tpm_tis_core tpm nfsd auth_rpcgss nfs_acl lockd grace sunrpc crc32c_intel serio_raw [last unloaded: i915]
> CPU: 3 PID: 1743 Comm: kworker/u8:22 Tainted: G W E 4.10.0-rc3ander+ #300
> Hardware name: Intel Corp. Geminilake/GLK RVP1 DDR4 (05), BIOS GELKRVPA.X64.0023.B40.1611302145 11/30/2016
> Workqueue: events_unbound async_run_entry_fn
> Call Trace:
> dump_stack+0x86/0xc3
> __warn+0xcb/0xf0
> warn_slowpath_fmt+0x5f/0x80
> verify_connector_state.isra.80+0x26c/0x2b0 [i915]
> intel_atomic_commit_tail+0x520/0x1000 [i915]
> ? remove_wait_queue+0x70/0x70
> intel_atomic_commit+0x3f8/0x520 [i915]
> ? intel_runtime_pm_put+0x6e/0xa0 [i915]
> drm_atomic_commit+0x4b/0x50 [drm]
> __intel_display_resume+0x72/0xc0 [i915]
> intel_display_resume+0x107/0x150 [i915]
> i915_drm_resume+0xe0/0x180 [i915]
> i915_pm_restore+0x1e/0x30 [i915]
> i915_pm_resume+0xe/0x10 [i915]
> pci_pm_resume+0x64/0xa0
> dpm_run_callback+0xa1/0x2a0
> ? pci_pm_thaw+0x90/0x90
> device_resume+0xe3/0x200
> async_resume+0x1d/0x50
> async_run_entry_fn+0x39/0x170
> process_one_work+0x212/0x670
> ? process_one_work+0x197/0x670
> worker_thread+0x4e/0x490
> kthread+0x101/0x140
> ? process_one_work+0x670/0x670
> ? kthread_create_on_node+0x60/0x60
> ret_from_fork+0x2a/0x40
>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
LGTM,
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 8795679..b729a39 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -469,14 +469,11 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> 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) | \
> - BIT_ULL(POWER_DOMAIN_INIT))
> + BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES))
> #define GLK_DISPLAY_DDI_B_POWER_DOMAINS ( \
> - BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) | \
> - BIT_ULL(POWER_DOMAIN_INIT))
> + BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES))
> #define GLK_DISPLAY_DDI_C_POWER_DOMAINS ( \
> - BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES) | \
> - BIT_ULL(POWER_DOMAIN_INIT))
> + BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES))
> #define GLK_DPIO_CMN_A_POWER_DOMAINS ( \
> BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) | \
> BIT_ULL(POWER_DOMAIN_AUX_A) | \
> --
> 2.9.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/6] drm/i915/glk: Don't attempt to sync DDI IO power well hw state
2017-02-10 13:29 [PATCH 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
` (3 preceding siblings ...)
2017-02-10 13:29 ` [PATCH 4/6] drm/i915/glk: Don't enable DDI IO power domains during init Ander Conselvan de Oliveira
@ 2017-02-10 13:29 ` Ander Conselvan de Oliveira
2017-02-16 10:19 ` Imre Deak
2017-02-10 13:29 ` [PATCH 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL Ander Conselvan de Oliveira
2017-02-10 14:26 ` ✗ Fi.CI.BAT: failure for Fix Geminilake DDI power well enable timeouts Patchwork
6 siblings, 1 reply; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-10 13:29 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
Geminilake's DDI IO power wells can only be enabled after a DPLL is
running and must be disabled before disabling the DPLL. Attempting to
change its state outside of this conditions will result in timeouts.
That is the case when intel_power_domains_sync_hw() is called from
intel_power_domains_init_hw(), where the attempt to disable the DDI IO
for an enabled port will timeout, so don't attempt to sync the hw state
for those power wells.
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index b729a39..4d2dc31 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2220,6 +2220,25 @@ static struct i915_power_well bxt_power_wells[] = {
},
};
+static void noop_power_well_sync_hw(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+}
+
+static const struct i915_power_well_ops glk_ddi_io_power_well_ops = {
+ /*
+ * DDI IO power wells in GLK can only be enabled after a DPLL is
+ * running and must be disabled before disabling the DPLL. Any
+ * attempt to change its state in different condidtions will lead
+ * to timeouts.
+ */
+ .sync_hw = noop_power_well_sync_hw,
+
+ .enable = skl_power_well_enable,
+ .disable = skl_power_well_disable,
+ .is_enabled = skl_power_well_enabled,
+};
+
static struct i915_power_well glk_power_wells[] = {
{
.name = "always-on",
@@ -2288,19 +2307,19 @@ static struct i915_power_well glk_power_wells[] = {
{
.name = "DDI A power well",
.domains = GLK_DISPLAY_DDI_A_POWER_DOMAINS,
- .ops = &skl_power_well_ops,
+ .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,
- .ops = &skl_power_well_ops,
+ .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,
- .ops = &skl_power_well_ops,
+ .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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 5/6] drm/i915/glk: Don't attempt to sync DDI IO power well hw state
2017-02-10 13:29 ` [PATCH 5/6] drm/i915/glk: Don't attempt to sync DDI IO power well hw state Ander Conselvan de Oliveira
@ 2017-02-16 10:19 ` Imre Deak
0 siblings, 0 replies; 17+ messages in thread
From: Imre Deak @ 2017-02-16 10:19 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
On Fri, Feb 10, 2017 at 03:29:58PM +0200, Ander Conselvan de Oliveira wrote:
> Geminilake's DDI IO power wells can only be enabled after a DPLL is
> running and must be disabled before disabling the DPLL. Attempting to
> change its state outside of this conditions will result in timeouts.
> That is the case when intel_power_domains_sync_hw() is called from
> intel_power_domains_init_hw(), where the attempt to disable the DDI IO
> for an enabled port will timeout, so don't attempt to sync the hw state
> for those power wells.
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
With my power well fixes patchset, this won't be needed, the existing
sync_hw hook will be equivalent with the BIOS request reg sanitization
added.
> ---
> drivers/gpu/drm/i915/intel_runtime_pm.c | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index b729a39..4d2dc31 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2220,6 +2220,25 @@ static struct i915_power_well bxt_power_wells[] = {
> },
> };
>
> +static void noop_power_well_sync_hw(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well)
> +{
> +}
> +
> +static const struct i915_power_well_ops glk_ddi_io_power_well_ops = {
> + /*
> + * DDI IO power wells in GLK can only be enabled after a DPLL is
> + * running and must be disabled before disabling the DPLL. Any
> + * attempt to change its state in different condidtions will lead
> + * to timeouts.
> + */
> + .sync_hw = noop_power_well_sync_hw,
> +
> + .enable = skl_power_well_enable,
> + .disable = skl_power_well_disable,
> + .is_enabled = skl_power_well_enabled,
> +};
> +
> static struct i915_power_well glk_power_wells[] = {
> {
> .name = "always-on",
> @@ -2288,19 +2307,19 @@ static struct i915_power_well glk_power_wells[] = {
> {
> .name = "DDI A power well",
> .domains = GLK_DISPLAY_DDI_A_POWER_DOMAINS,
> - .ops = &skl_power_well_ops,
> + .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,
> - .ops = &skl_power_well_ops,
> + .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,
> - .ops = &skl_power_well_ops,
> + .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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL
2017-02-10 13:29 [PATCH 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
` (4 preceding siblings ...)
2017-02-10 13:29 ` [PATCH 5/6] drm/i915/glk: Don't attempt to sync DDI IO power well hw state Ander Conselvan de Oliveira
@ 2017-02-10 13:29 ` Ander Conselvan de Oliveira
2017-02-13 16:32 ` David Weinehall
2017-02-16 10:16 ` Imre Deak
2017-02-10 14:26 ` ✗ Fi.CI.BAT: failure for Fix Geminilake DDI power well enable timeouts Patchwork
6 siblings, 2 replies; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-10 13:29 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
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.weinehall@linux.intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@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);
+ }
+
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL
2017-02-10 13:29 ` [PATCH 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL Ander Conselvan de Oliveira
@ 2017-02-13 16:32 ` David Weinehall
2017-02-16 10:16 ` Imre Deak
1 sibling, 0 replies; 17+ messages in thread
From: David Weinehall @ 2017-02-13 16:32 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
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.weinehall@linux.intel.com>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: David Weinehall <david.weinehall@linux.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);
> + }
> +
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL
2017-02-10 13:29 ` [PATCH 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL Ander Conselvan de Oliveira
2017-02-13 16:32 ` David Weinehall
@ 2017-02-16 10:16 ` Imre Deak
1 sibling, 0 replies; 17+ messages in thread
From: Imre Deak @ 2017-02-16 10:16 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
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.weinehall@linux.intel.com>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@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
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✗ Fi.CI.BAT: failure for Fix Geminilake DDI power well enable timeouts
2017-02-10 13:29 [PATCH 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
` (5 preceding siblings ...)
2017-02-10 13:29 ` [PATCH 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL Ander Conselvan de Oliveira
@ 2017-02-10 14:26 ` Patchwork
6 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2017-02-10 14:26 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
== Series Details ==
Series: Fix Geminilake DDI power well enable timeouts
URL : https://patchwork.freedesktop.org/series/19451/
State : failure
== Summary ==
Series 19451v1 Fix Geminilake DDI power well enable timeouts
https://patchwork.freedesktop.org/api/1.0/series/19451/revisions/1/mbox/
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> DMESG-WARN (fi-hsw-4770)
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-hsw-4770)
Test kms_busy:
Subgroup basic-flip-default-a:
pass -> DMESG-WARN (fi-hsw-4770)
Subgroup basic-flip-default-b:
pass -> DMESG-WARN (fi-hsw-4770)
Subgroup basic-flip-default-c:
pass -> DMESG-WARN (fi-hsw-4770)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
pass -> INCOMPLETE (fi-hsw-4770)
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-a-frame-sequence:
pass -> FAIL (fi-skl-6770hq)
Subgroup read-crc-pipe-c-frame-sequence:
pass -> FAIL (fi-skl-6770hq)
Test pm_rpm:
Subgroup basic-pci-d3-state:
fail -> PASS (fi-kbl-7500u)
Subgroup basic-rte:
fail -> PASS (fi-kbl-7500u)
fi-bdw-5557u total:252 pass:241 dwarn:0 dfail:0 fail:0 skip:11
fi-bsw-n3050 total:252 pass:213 dwarn:0 dfail:0 fail:0 skip:39
fi-bxt-j4205 total:252 pass:233 dwarn:0 dfail:0 fail:0 skip:19
fi-bxt-t5700 total:83 pass:70 dwarn:0 dfail:0 fail:0 skip:12
fi-byt-j1900 total:252 pass:225 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-n2820 total:252 pass:221 dwarn:0 dfail:0 fail:0 skip:31
fi-hsw-4770 total:173 pass:158 dwarn:5 dfail:0 fail:0 skip:9
fi-hsw-4770r total:252 pass:236 dwarn:0 dfail:0 fail:0 skip:16
fi-ilk-650 total:252 pass:202 dwarn:0 dfail:0 fail:0 skip:50
fi-ivb-3520m total:252 pass:234 dwarn:0 dfail:0 fail:0 skip:18
fi-ivb-3770 total:252 pass:234 dwarn:0 dfail:0 fail:0 skip:18
fi-kbl-7500u total:252 pass:234 dwarn:0 dfail:0 fail:0 skip:18
fi-skl-6260u total:252 pass:242 dwarn:0 dfail:0 fail:0 skip:10
fi-skl-6700hq total:252 pass:235 dwarn:0 dfail:0 fail:0 skip:17
fi-skl-6700k total:252 pass:230 dwarn:4 dfail:0 fail:0 skip:18
fi-skl-6770hq total:252 pass:240 dwarn:0 dfail:0 fail:2 skip:10
fi-snb-2520m total:252 pass:224 dwarn:0 dfail:0 fail:0 skip:28
fi-snb-2600 total:252 pass:223 dwarn:0 dfail:0 fail:0 skip:29
c46c80c1b1cdc18ed3d44ce289032946b48454c6 drm-tip: 2017y-02m-10d-09h-45m-22s UTC integration manifest
2cfe1b1 drm/i915: Only enable DDI IO power domains after enabling DPLL
10a113c drm/i915/glk: Don't attempt to sync DDI IO power well hw state
6b59d79 drm/i915/glk: Don't enable DDI IO power domains during init
f3172cc drm/i915/glk: Implement WaDDIIOTimeout
a27887b drm/i915: Store encoder power domain in struct intel_encoder
ef50071 drm/i915: Store aux power domain in intel_dp
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3766/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread