| Message ID | 20170210132959.16594-2-ander.conselvan.de.oliveira@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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);
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(-)