| Message ID | 1447084107-8521-4-git-send-email-patrik.jakobsson@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Introduce intel_display_port_aux_power_domain() which simply returns > the appropriate AUX power domain for a specific port, and then > replace > the intel_display_port_power_domain() with calls to the new function > in the DP code. As long as we're not actually enabling the port we > don't > need the lane power domains, and those are handled now purely from > modeset_update_crtc_power_domains(). > > My initial motivation for this was to see if I could keep the DPIO > power > wells powered down while doing AUX on CHV, but turns out I can't so > this > doesn't change anything for CHV at least. But I think it's still a > worthwile change. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 40 > ++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_dp.c | 48 +++++++++++--------------- > ---------- > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > 3 files changed, 56 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index d0fec07..c2578d9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5143,6 +5143,23 @@ 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; > + default: > + WARN_ON_ONCE(1); > + return POWER_DOMAIN_AUX_A; > + } > +} Looks like port E is missing. I chatted with Ville he has some idea to fix this. > + > #define for_each_power_domain(domain, mask) > \ > for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) > \ > if ((1 << (domain)) & (mask)) > @@ -5174,6 +5191,29 @@ 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_device *dev = 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)); > + case INTEL_OUTPUT_DISPLAYPORT: > + 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: > + WARN_ON_ONCE(1); > + return POWER_DOMAIN_AUX_A; > + } > +} > + > static unsigned long get_crtc_power_domains(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > diff --git a/drivers/gpu/drm/i915/intel_dp.c > b/drivers/gpu/drm/i915/intel_dp.c > index 4655af0..3978540 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -277,7 +277,7 @@ static void pps_lock(struct intel_dp *intel_dp) > * See vlv_power_sequencer_reset() why we need > * a power domain reference here. > */ > - power_domain = intel_display_port_power_domain(encoder); > + power_domain = intel_display_port_aux_power_domain(encoder); > intel_display_power_get(dev_priv, power_domain); > > mutex_lock(&dev_priv->pps_mutex); > @@ -293,7 +293,7 @@ static void pps_unlock(struct intel_dp *intel_dp) > > mutex_unlock(&dev_priv->pps_mutex); > > - power_domain = intel_display_port_power_domain(encoder); > + power_domain = intel_display_port_aux_power_domain(encoder); > intel_display_power_put(dev_priv, power_domain); > } > > @@ -816,8 +816,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > > intel_dp_check_edp(intel_dp); > > - intel_aux_display_runtime_get(dev_priv); > - > /* Try to wait for any previous AUX channel activity */ > for (try = 0; try < 3; try++) { > status = I915_READ_NOTRACE(ch_ctl); > @@ -926,7 +924,6 @@ done: > ret = recv_bytes; > out: > pm_qos_update_request(&dev_priv->pm_qos, > PM_QOS_DEFAULT_VALUE); > - intel_aux_display_runtime_put(dev_priv); > > if (vdd) > edp_panel_vdd_off(intel_dp, false); > @@ -1784,7 +1781,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_power_domain(intel_encoder); > + power_domain = > intel_display_port_aux_power_domain(intel_encoder); > intel_display_power_get(dev_priv, power_domain); > > DRM_DEBUG_KMS("Turning eDP port %c VDD on\n", > @@ -1874,7 +1871,7 @@ static void edp_panel_vdd_off_sync(struct > intel_dp *intel_dp) > if ((pp & POWER_TARGET_ON) == 0) > intel_dp->last_power_cycle = jiffies; > > - power_domain = > intel_display_port_power_domain(intel_encoder); > + power_domain = > intel_display_port_aux_power_domain(intel_encoder); > intel_display_power_put(dev_priv, power_domain); > } > > @@ -2025,7 +2022,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_power_domain(intel_encoder); > + power_domain = > intel_display_port_aux_power_domain(intel_encoder); > intel_display_power_put(dev_priv, power_domain); > } > > @@ -4765,26 +4762,6 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) > intel_dp->has_audio = false; > } > > -static enum intel_display_power_domain > -intel_dp_power_get(struct intel_dp *dp) > -{ > - struct intel_encoder *encoder = &dp_to_dig_port(dp)->base; > - enum intel_display_power_domain power_domain; > - > - power_domain = intel_display_port_power_domain(encoder); > - intel_display_power_get(to_i915(encoder->base.dev), > power_domain); > - > - return power_domain; > -} > - > -static void > -intel_dp_power_put(struct intel_dp *dp, > - enum intel_display_power_domain power_domain) > -{ > - struct intel_encoder *encoder = &dp_to_dig_port(dp)->base; > - intel_display_power_put(to_i915(encoder->base.dev), > power_domain); > -} > - > static enum drm_connector_status > intel_dp_detect(struct drm_connector *connector, bool force) > { > @@ -4808,7 +4785,8 @@ intel_dp_detect(struct drm_connector > *connector, bool force) > return connector_status_disconnected; > } > > - power_domain = intel_dp_power_get(intel_dp); > + power_domain = > intel_display_port_aux_power_domain(intel_encoder); > + intel_display_power_get(to_i915(dev), power_domain); > > /* Can't disconnect eDP, but you can close the lid... */ > if (is_edp(intel_dp)) > @@ -4853,7 +4831,7 @@ intel_dp_detect(struct drm_connector > *connector, bool force) > } > > out: > - intel_dp_power_put(intel_dp, power_domain); > + intel_display_power_put(to_i915(dev), power_domain); > return status; > } > > @@ -4862,6 +4840,7 @@ 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", > @@ -4871,11 +4850,12 @@ intel_dp_force(struct drm_connector > *connector) > if (connector->status != connector_status_connected) > return; > > - power_domain = intel_dp_power_get(intel_dp); > + power_domain = > intel_display_port_aux_power_domain(intel_encoder); > + intel_display_power_get(dev_priv, power_domain); > > intel_dp_set_edid(intel_dp); > > - intel_dp_power_put(intel_dp, power_domain); > + intel_display_power_put(dev_priv, power_domain); > > if (intel_encoder->type != INTEL_OUTPUT_EDP) > intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > @@ -5091,7 +5071,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_power_domain(&intel_dig_port->base); > + power_domain = > intel_display_port_aux_power_domain(&intel_dig_port->base); > intel_display_power_get(dev_priv, power_domain); > > edp_panel_vdd_schedule_off(intel_dp); > @@ -5172,7 +5152,7 @@ intel_dp_hpd_pulse(struct intel_digital_port > *intel_dig_port, bool long_hpd) > port_name(intel_dig_port->port), > long_hpd ? "long" : "short"); > > - power_domain = > intel_display_port_power_domain(intel_encoder); > + power_domain = > intel_display_port_aux_power_domain(intel_encoder); > intel_display_power_get(dev_priv, power_domain); > > if (long_hpd) { > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index a68b6cd..7d11aa0 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1188,6 +1188,8 @@ 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); > void intel_modeset_preclose(struct drm_device *dev, struct drm_file > *file);
On Wed, Nov 11, 2015 at 08:22:03PM +0200, Imre Deak wrote: > On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Introduce intel_display_port_aux_power_domain() which simply returns > > the appropriate AUX power domain for a specific port, and then > > replace > > the intel_display_port_power_domain() with calls to the new function > > in the DP code. As long as we're not actually enabling the port we > > don't > > need the lane power domains, and those are handled now purely from > > modeset_update_crtc_power_domains(). > > > > My initial motivation for this was to see if I could keep the DPIO > > power > > wells powered down while doing AUX on CHV, but turns out I can't so > > this > > doesn't change anything for CHV at least. But I think it's still a > > worthwile change. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 40 > > ++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_dp.c | 48 +++++++++++--------------- > > ---------- > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > 3 files changed, 56 insertions(+), 34 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index d0fec07..c2578d9 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -5143,6 +5143,23 @@ 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; > > + default: > > + WARN_ON_ONCE(1); > > + return POWER_DOMAIN_AUX_A; > > + } > > +} > > Looks like port E is missing. I chatted with Ville he has some idea to > fix this. Yeah, so there's no dedicated AUX block for port E, and instead VBT tells us which AUX block to use. The current code doin that is rather messy, but I have a cleaned it up during my register type-safety journey. I just reposted the remaining AUX related patches [1] from that series. So I was thinking that we could include an 'enum port aux_port' inside intel_dp, and use that to pick the right power domain. [1] http://lists.freedesktop.org/archives/intel-gfx/2015-November/079918.html > > > + > > #define for_each_power_domain(domain, mask) > > \ > > for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) > > \ > > if ((1 << (domain)) & (mask)) > > @@ -5174,6 +5191,29 @@ 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_device *dev = 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)); > > + case INTEL_OUTPUT_DISPLAYPORT: > > + 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: > > + WARN_ON_ONCE(1); > > + return POWER_DOMAIN_AUX_A; > > + } > > +} > > + > > static unsigned long get_crtc_power_domains(struct drm_crtc *crtc) > > { > > struct drm_device *dev = crtc->dev; > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index 4655af0..3978540 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -277,7 +277,7 @@ static void pps_lock(struct intel_dp *intel_dp) > > * See vlv_power_sequencer_reset() why we need > > * a power domain reference here. > > */ > > - power_domain = intel_display_port_power_domain(encoder); > > + power_domain = intel_display_port_aux_power_domain(encoder); > > intel_display_power_get(dev_priv, power_domain); > > > > mutex_lock(&dev_priv->pps_mutex); > > @@ -293,7 +293,7 @@ static void pps_unlock(struct intel_dp *intel_dp) > > > > mutex_unlock(&dev_priv->pps_mutex); > > > > - power_domain = intel_display_port_power_domain(encoder); > > + power_domain = intel_display_port_aux_power_domain(encoder); > > intel_display_power_put(dev_priv, power_domain); > > } > > > > @@ -816,8 +816,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > > > > intel_dp_check_edp(intel_dp); > > > > - intel_aux_display_runtime_get(dev_priv); > > - > > /* Try to wait for any previous AUX channel activity */ > > for (try = 0; try < 3; try++) { > > status = I915_READ_NOTRACE(ch_ctl); > > @@ -926,7 +924,6 @@ done: > > ret = recv_bytes; > > out: > > pm_qos_update_request(&dev_priv->pm_qos, > > PM_QOS_DEFAULT_VALUE); > > - intel_aux_display_runtime_put(dev_priv); > > > > if (vdd) > > edp_panel_vdd_off(intel_dp, false); > > @@ -1784,7 +1781,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_power_domain(intel_encoder); > > + power_domain = > > intel_display_port_aux_power_domain(intel_encoder); > > intel_display_power_get(dev_priv, power_domain); > > > > DRM_DEBUG_KMS("Turning eDP port %c VDD on\n", > > @@ -1874,7 +1871,7 @@ static void edp_panel_vdd_off_sync(struct > > intel_dp *intel_dp) > > if ((pp & POWER_TARGET_ON) == 0) > > intel_dp->last_power_cycle = jiffies; > > > > - power_domain = > > intel_display_port_power_domain(intel_encoder); > > + power_domain = > > intel_display_port_aux_power_domain(intel_encoder); > > intel_display_power_put(dev_priv, power_domain); > > } > > > > @@ -2025,7 +2022,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_power_domain(intel_encoder); > > + power_domain = > > intel_display_port_aux_power_domain(intel_encoder); > > intel_display_power_put(dev_priv, power_domain); > > } > > > > @@ -4765,26 +4762,6 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) > > intel_dp->has_audio = false; > > } > > > > -static enum intel_display_power_domain > > -intel_dp_power_get(struct intel_dp *dp) > > -{ > > - struct intel_encoder *encoder = &dp_to_dig_port(dp)->base; > > - enum intel_display_power_domain power_domain; > > - > > - power_domain = intel_display_port_power_domain(encoder); > > - intel_display_power_get(to_i915(encoder->base.dev), > > power_domain); > > - > > - return power_domain; > > -} > > - > > -static void > > -intel_dp_power_put(struct intel_dp *dp, > > - enum intel_display_power_domain power_domain) > > -{ > > - struct intel_encoder *encoder = &dp_to_dig_port(dp)->base; > > - intel_display_power_put(to_i915(encoder->base.dev), > > power_domain); > > -} > > - > > static enum drm_connector_status > > intel_dp_detect(struct drm_connector *connector, bool force) > > { > > @@ -4808,7 +4785,8 @@ intel_dp_detect(struct drm_connector > > *connector, bool force) > > return connector_status_disconnected; > > } > > > > - power_domain = intel_dp_power_get(intel_dp); > > + power_domain = > > intel_display_port_aux_power_domain(intel_encoder); > > + intel_display_power_get(to_i915(dev), power_domain); > > > > /* Can't disconnect eDP, but you can close the lid... */ > > if (is_edp(intel_dp)) > > @@ -4853,7 +4831,7 @@ intel_dp_detect(struct drm_connector > > *connector, bool force) > > } > > > > out: > > - intel_dp_power_put(intel_dp, power_domain); > > + intel_display_power_put(to_i915(dev), power_domain); > > return status; > > } > > > > @@ -4862,6 +4840,7 @@ 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", > > @@ -4871,11 +4850,12 @@ intel_dp_force(struct drm_connector > > *connector) > > if (connector->status != connector_status_connected) > > return; > > > > - power_domain = intel_dp_power_get(intel_dp); > > + power_domain = > > intel_display_port_aux_power_domain(intel_encoder); > > + intel_display_power_get(dev_priv, power_domain); > > > > intel_dp_set_edid(intel_dp); > > > > - intel_dp_power_put(intel_dp, power_domain); > > + intel_display_power_put(dev_priv, power_domain); > > > > if (intel_encoder->type != INTEL_OUTPUT_EDP) > > intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > @@ -5091,7 +5071,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_power_domain(&intel_dig_port->base); > > + power_domain = > > intel_display_port_aux_power_domain(&intel_dig_port->base); > > intel_display_power_get(dev_priv, power_domain); > > > > edp_panel_vdd_schedule_off(intel_dp); > > @@ -5172,7 +5152,7 @@ intel_dp_hpd_pulse(struct intel_digital_port > > *intel_dig_port, bool long_hpd) > > port_name(intel_dig_port->port), > > long_hpd ? "long" : "short"); > > > > - power_domain = > > intel_display_port_power_domain(intel_encoder); > > + power_domain = > > intel_display_port_aux_power_domain(intel_encoder); > > intel_display_power_get(dev_priv, power_domain); > > > > if (long_hpd) { > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index a68b6cd..7d11aa0 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1188,6 +1188,8 @@ 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); > > void intel_modeset_preclose(struct drm_device *dev, struct drm_file > > *file);
On Wed, Nov 11, 2015 at 08:37:36PM +0200, Ville Syrjälä wrote: > On Wed, Nov 11, 2015 at 08:22:03PM +0200, Imre Deak wrote: > > On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Introduce intel_display_port_aux_power_domain() which simply returns > > > the appropriate AUX power domain for a specific port, and then > > > replace > > > the intel_display_port_power_domain() with calls to the new function > > > in the DP code. As long as we're not actually enabling the port we > > > don't > > > need the lane power domains, and those are handled now purely from > > > modeset_update_crtc_power_domains(). > > > > > > My initial motivation for this was to see if I could keep the DPIO > > > power > > > wells powered down while doing AUX on CHV, but turns out I can't so > > > this > > > doesn't change anything for CHV at least. But I think it's still a > > > worthwile change. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 40 > > > ++++++++++++++++++++++++++++++ > > > drivers/gpu/drm/i915/intel_dp.c | 48 +++++++++++--------------- > > > ---------- > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > > 3 files changed, 56 insertions(+), 34 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index d0fec07..c2578d9 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -5143,6 +5143,23 @@ 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; > > > + default: > > > + WARN_ON_ONCE(1); > > > + return POWER_DOMAIN_AUX_A; > > > + } > > > +} > > > > Looks like port E is missing. I chatted with Ville he has some idea to > > fix this. > > Yeah, so there's no dedicated AUX block for port E, and instead VBT > tells us which AUX block to use. The current code doin that is rather > messy, but I have a cleaned it up during my register type-safety > journey. I just reposted the remaining AUX related patches [1] from > that series. > > So I was thinking that we could include an 'enum port aux_port' inside > intel_dp, and use that to pick the right power domain. > > [1] http://lists.freedesktop.org/archives/intel-gfx/2015-November/079918.html Ok so one of Aux A-D is hardwired to port E and only VBT can tell us which? Do we need to change anything in this patch or can we add the aux_port enum later on? > > > > > > + > > > #define for_each_power_domain(domain, mask) > > > \ > > > for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) > > > \ > > > if ((1 << (domain)) & (mask)) > > > @@ -5174,6 +5191,29 @@ 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_device *dev = 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)); > > > + case INTEL_OUTPUT_DISPLAYPORT: > > > + 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: > > > + WARN_ON_ONCE(1); > > > + return POWER_DOMAIN_AUX_A; > > > + } > > > +} > > > + > > > static unsigned long get_crtc_power_domains(struct drm_crtc *crtc) > > > { > > > struct drm_device *dev = crtc->dev; > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > b/drivers/gpu/drm/i915/intel_dp.c > > > index 4655af0..3978540 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -277,7 +277,7 @@ static void pps_lock(struct intel_dp *intel_dp) > > > * See vlv_power_sequencer_reset() why we need > > > * a power domain reference here. > > > */ > > > - power_domain = intel_display_port_power_domain(encoder); > > > + power_domain = intel_display_port_aux_power_domain(encoder); > > > intel_display_power_get(dev_priv, power_domain); > > > > > > mutex_lock(&dev_priv->pps_mutex); > > > @@ -293,7 +293,7 @@ static void pps_unlock(struct intel_dp *intel_dp) > > > > > > mutex_unlock(&dev_priv->pps_mutex); > > > > > > - power_domain = intel_display_port_power_domain(encoder); > > > + power_domain = intel_display_port_aux_power_domain(encoder); > > > intel_display_power_put(dev_priv, power_domain); > > > } > > > > > > @@ -816,8 +816,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > > > > > > intel_dp_check_edp(intel_dp); > > > > > > - intel_aux_display_runtime_get(dev_priv); > > > - > > > /* Try to wait for any previous AUX channel activity */ > > > for (try = 0; try < 3; try++) { > > > status = I915_READ_NOTRACE(ch_ctl); > > > @@ -926,7 +924,6 @@ done: > > > ret = recv_bytes; > > > out: > > > pm_qos_update_request(&dev_priv->pm_qos, > > > PM_QOS_DEFAULT_VALUE); > > > - intel_aux_display_runtime_put(dev_priv); > > > > > > if (vdd) > > > edp_panel_vdd_off(intel_dp, false); > > > @@ -1784,7 +1781,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_power_domain(intel_encoder); > > > + power_domain = > > > intel_display_port_aux_power_domain(intel_encoder); > > > intel_display_power_get(dev_priv, power_domain); > > > > > > DRM_DEBUG_KMS("Turning eDP port %c VDD on\n", > > > @@ -1874,7 +1871,7 @@ static void edp_panel_vdd_off_sync(struct > > > intel_dp *intel_dp) > > > if ((pp & POWER_TARGET_ON) == 0) > > > intel_dp->last_power_cycle = jiffies; > > > > > > - power_domain = > > > intel_display_port_power_domain(intel_encoder); > > > + power_domain = > > > intel_display_port_aux_power_domain(intel_encoder); > > > intel_display_power_put(dev_priv, power_domain); > > > } > > > > > > @@ -2025,7 +2022,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_power_domain(intel_encoder); > > > + power_domain = > > > intel_display_port_aux_power_domain(intel_encoder); > > > intel_display_power_put(dev_priv, power_domain); > > > } > > > > > > @@ -4765,26 +4762,6 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) > > > intel_dp->has_audio = false; > > > } > > > > > > -static enum intel_display_power_domain > > > -intel_dp_power_get(struct intel_dp *dp) > > > -{ > > > - struct intel_encoder *encoder = &dp_to_dig_port(dp)->base; > > > - enum intel_display_power_domain power_domain; > > > - > > > - power_domain = intel_display_port_power_domain(encoder); > > > - intel_display_power_get(to_i915(encoder->base.dev), > > > power_domain); > > > - > > > - return power_domain; > > > -} > > > - > > > -static void > > > -intel_dp_power_put(struct intel_dp *dp, > > > - enum intel_display_power_domain power_domain) > > > -{ > > > - struct intel_encoder *encoder = &dp_to_dig_port(dp)->base; > > > - intel_display_power_put(to_i915(encoder->base.dev), > > > power_domain); > > > -} > > > - > > > static enum drm_connector_status > > > intel_dp_detect(struct drm_connector *connector, bool force) > > > { > > > @@ -4808,7 +4785,8 @@ intel_dp_detect(struct drm_connector > > > *connector, bool force) > > > return connector_status_disconnected; > > > } > > > > > > - power_domain = intel_dp_power_get(intel_dp); > > > + power_domain = > > > intel_display_port_aux_power_domain(intel_encoder); > > > + intel_display_power_get(to_i915(dev), power_domain); > > > > > > /* Can't disconnect eDP, but you can close the lid... */ > > > if (is_edp(intel_dp)) > > > @@ -4853,7 +4831,7 @@ intel_dp_detect(struct drm_connector > > > *connector, bool force) > > > } > > > > > > out: > > > - intel_dp_power_put(intel_dp, power_domain); > > > + intel_display_power_put(to_i915(dev), power_domain); > > > return status; > > > } > > > > > > @@ -4862,6 +4840,7 @@ 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", > > > @@ -4871,11 +4850,12 @@ intel_dp_force(struct drm_connector > > > *connector) > > > if (connector->status != connector_status_connected) > > > return; > > > > > > - power_domain = intel_dp_power_get(intel_dp); > > > + power_domain = > > > intel_display_port_aux_power_domain(intel_encoder); > > > + intel_display_power_get(dev_priv, power_domain); > > > > > > intel_dp_set_edid(intel_dp); > > > > > > - intel_dp_power_put(intel_dp, power_domain); > > > + intel_display_power_put(dev_priv, power_domain); > > > > > > if (intel_encoder->type != INTEL_OUTPUT_EDP) > > > intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > > @@ -5091,7 +5071,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_power_domain(&intel_dig_port->base); > > > + power_domain = > > > intel_display_port_aux_power_domain(&intel_dig_port->base); > > > intel_display_power_get(dev_priv, power_domain); > > > > > > edp_panel_vdd_schedule_off(intel_dp); > > > @@ -5172,7 +5152,7 @@ intel_dp_hpd_pulse(struct intel_digital_port > > > *intel_dig_port, bool long_hpd) > > > port_name(intel_dig_port->port), > > > long_hpd ? "long" : "short"); > > > > > > - power_domain = > > > intel_display_port_power_domain(intel_encoder); > > > + power_domain = > > > intel_display_port_aux_power_domain(intel_encoder); > > > intel_display_power_get(dev_priv, power_domain); > > > > > > if (long_hpd) { > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > > b/drivers/gpu/drm/i915/intel_drv.h > > > index a68b6cd..7d11aa0 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -1188,6 +1188,8 @@ 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); > > > void intel_modeset_preclose(struct drm_device *dev, struct drm_file > > > *file); > > -- > Ville Syrjälä > Intel OTC
On Thu, Nov 12, 2015 at 10:02:36AM +0100, Patrik Jakobsson wrote: > On Wed, Nov 11, 2015 at 08:37:36PM +0200, Ville Syrjälä wrote: > > On Wed, Nov 11, 2015 at 08:22:03PM +0200, Imre Deak wrote: > > > On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote: > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > Introduce intel_display_port_aux_power_domain() which simply returns > > > > the appropriate AUX power domain for a specific port, and then > > > > replace > > > > the intel_display_port_power_domain() with calls to the new function > > > > in the DP code. As long as we're not actually enabling the port we > > > > don't > > > > need the lane power domains, and those are handled now purely from > > > > modeset_update_crtc_power_domains(). > > > > > > > > My initial motivation for this was to see if I could keep the DPIO > > > > power > > > > wells powered down while doing AUX on CHV, but turns out I can't so > > > > this > > > > doesn't change anything for CHV at least. But I think it's still a > > > > worthwile change. > > > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > > > --- > > > > drivers/gpu/drm/i915/intel_display.c | 40 > > > > ++++++++++++++++++++++++++++++ > > > > drivers/gpu/drm/i915/intel_dp.c | 48 +++++++++++--------------- > > > > ---------- > > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > > > 3 files changed, 56 insertions(+), 34 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > > b/drivers/gpu/drm/i915/intel_display.c > > > > index d0fec07..c2578d9 100644 > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > @@ -5143,6 +5143,23 @@ 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; > > > > + default: > > > > + WARN_ON_ONCE(1); > > > > + return POWER_DOMAIN_AUX_A; > > > > + } > > > > +} > > > > > > Looks like port E is missing. I chatted with Ville he has some idea to > > > fix this. > > > > Yeah, so there's no dedicated AUX block for port E, and instead VBT > > tells us which AUX block to use. The current code doin that is rather > > messy, but I have a cleaned it up during my register type-safety > > journey. I just reposted the remaining AUX related patches [1] from > > that series. > > > > So I was thinking that we could include an 'enum port aux_port' inside > > intel_dp, and use that to pick the right power domain. > > > > [1] http://lists.freedesktop.org/archives/intel-gfx/2015-November/079918.html > > Ok so one of Aux A-D is hardwired to port E and only VBT can tell us which? Yep. > Do > we need to change anything in this patch or can we add the aux_port enum later > on? Hmm. Lemme think. Before, we too the port power domain, so DDI_E, and after the patch we would hit the default: case and get the WARN. So that would be a regression of sorts. I guess as a quick hack you could return eg. POWER_DOMAIN_AUX_D for port E, and add a FIXME that it needs to be fixed, if you don't want to tackle the aux_port idea (or something silimar right now). On a side note maybe we should add MISSING_CASE_ONCE() to get some better debug output from these kinds of cases... > > > > > > > > > > + > > > > #define for_each_power_domain(domain, mask) > > > > \ > > > > for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) > > > > \ > > > > if ((1 << (domain)) & (mask)) > > > > @@ -5174,6 +5191,29 @@ 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_device *dev = 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)); > > > > + case INTEL_OUTPUT_DISPLAYPORT: > > > > + 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: > > > > + WARN_ON_ONCE(1); > > > > + return POWER_DOMAIN_AUX_A; > > > > + } > > > > +} > > > > + > > > > static unsigned long get_crtc_power_domains(struct drm_crtc *crtc) > > > > { > > > > struct drm_device *dev = crtc->dev; > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > > b/drivers/gpu/drm/i915/intel_dp.c > > > > index 4655af0..3978540 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > @@ -277,7 +277,7 @@ static void pps_lock(struct intel_dp *intel_dp) > > > > * See vlv_power_sequencer_reset() why we need > > > > * a power domain reference here. > > > > */ > > > > - power_domain = intel_display_port_power_domain(encoder); > > > > + power_domain = intel_display_port_aux_power_domain(encoder); > > > > intel_display_power_get(dev_priv, power_domain); > > > > > > > > mutex_lock(&dev_priv->pps_mutex); > > > > @@ -293,7 +293,7 @@ static void pps_unlock(struct intel_dp *intel_dp) > > > > > > > > mutex_unlock(&dev_priv->pps_mutex); > > > > > > > > - power_domain = intel_display_port_power_domain(encoder); > > > > + power_domain = intel_display_port_aux_power_domain(encoder); > > > > intel_display_power_put(dev_priv, power_domain); > > > > } > > > > > > > > @@ -816,8 +816,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > > > > > > > > intel_dp_check_edp(intel_dp); > > > > > > > > - intel_aux_display_runtime_get(dev_priv); > > > > - > > > > /* Try to wait for any previous AUX channel activity */ > > > > for (try = 0; try < 3; try++) { > > > > status = I915_READ_NOTRACE(ch_ctl); > > > > @@ -926,7 +924,6 @@ done: > > > > ret = recv_bytes; > > > > out: > > > > pm_qos_update_request(&dev_priv->pm_qos, > > > > PM_QOS_DEFAULT_VALUE); > > > > - intel_aux_display_runtime_put(dev_priv); > > > > > > > > if (vdd) > > > > edp_panel_vdd_off(intel_dp, false); > > > > @@ -1784,7 +1781,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_power_domain(intel_encoder); > > > > + power_domain = > > > > intel_display_port_aux_power_domain(intel_encoder); > > > > intel_display_power_get(dev_priv, power_domain); > > > > > > > > DRM_DEBUG_KMS("Turning eDP port %c VDD on\n", > > > > @@ -1874,7 +1871,7 @@ static void edp_panel_vdd_off_sync(struct > > > > intel_dp *intel_dp) > > > > if ((pp & POWER_TARGET_ON) == 0) > > > > intel_dp->last_power_cycle = jiffies; > > > > > > > > - power_domain = > > > > intel_display_port_power_domain(intel_encoder); > > > > + power_domain = > > > > intel_display_port_aux_power_domain(intel_encoder); > > > > intel_display_power_put(dev_priv, power_domain); > > > > } > > > > > > > > @@ -2025,7 +2022,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_power_domain(intel_encoder); > > > > + power_domain = > > > > intel_display_port_aux_power_domain(intel_encoder); > > > > intel_display_power_put(dev_priv, power_domain); > > > > } > > > > > > > > @@ -4765,26 +4762,6 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) > > > > intel_dp->has_audio = false; > > > > } > > > > > > > > -static enum intel_display_power_domain > > > > -intel_dp_power_get(struct intel_dp *dp) > > > > -{ > > > > - struct intel_encoder *encoder = &dp_to_dig_port(dp)->base; > > > > - enum intel_display_power_domain power_domain; > > > > - > > > > - power_domain = intel_display_port_power_domain(encoder); > > > > - intel_display_power_get(to_i915(encoder->base.dev), > > > > power_domain); > > > > - > > > > - return power_domain; > > > > -} > > > > - > > > > -static void > > > > -intel_dp_power_put(struct intel_dp *dp, > > > > - enum intel_display_power_domain power_domain) > > > > -{ > > > > - struct intel_encoder *encoder = &dp_to_dig_port(dp)->base; > > > > - intel_display_power_put(to_i915(encoder->base.dev), > > > > power_domain); > > > > -} > > > > - > > > > static enum drm_connector_status > > > > intel_dp_detect(struct drm_connector *connector, bool force) > > > > { > > > > @@ -4808,7 +4785,8 @@ intel_dp_detect(struct drm_connector > > > > *connector, bool force) > > > > return connector_status_disconnected; > > > > } > > > > > > > > - power_domain = intel_dp_power_get(intel_dp); > > > > + power_domain = > > > > intel_display_port_aux_power_domain(intel_encoder); > > > > + intel_display_power_get(to_i915(dev), power_domain); > > > > > > > > /* Can't disconnect eDP, but you can close the lid... */ > > > > if (is_edp(intel_dp)) > > > > @@ -4853,7 +4831,7 @@ intel_dp_detect(struct drm_connector > > > > *connector, bool force) > > > > } > > > > > > > > out: > > > > - intel_dp_power_put(intel_dp, power_domain); > > > > + intel_display_power_put(to_i915(dev), power_domain); > > > > return status; > > > > } > > > > > > > > @@ -4862,6 +4840,7 @@ 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", > > > > @@ -4871,11 +4850,12 @@ intel_dp_force(struct drm_connector > > > > *connector) > > > > if (connector->status != connector_status_connected) > > > > return; > > > > > > > > - power_domain = intel_dp_power_get(intel_dp); > > > > + power_domain = > > > > intel_display_port_aux_power_domain(intel_encoder); > > > > + intel_display_power_get(dev_priv, power_domain); > > > > > > > > intel_dp_set_edid(intel_dp); > > > > > > > > - intel_dp_power_put(intel_dp, power_domain); > > > > + intel_display_power_put(dev_priv, power_domain); > > > > > > > > if (intel_encoder->type != INTEL_OUTPUT_EDP) > > > > intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > > > @@ -5091,7 +5071,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_power_domain(&intel_dig_port->base); > > > > + power_domain = > > > > intel_display_port_aux_power_domain(&intel_dig_port->base); > > > > intel_display_power_get(dev_priv, power_domain); > > > > > > > > edp_panel_vdd_schedule_off(intel_dp); > > > > @@ -5172,7 +5152,7 @@ intel_dp_hpd_pulse(struct intel_digital_port > > > > *intel_dig_port, bool long_hpd) > > > > port_name(intel_dig_port->port), > > > > long_hpd ? "long" : "short"); > > > > > > > > - power_domain = > > > > intel_display_port_power_domain(intel_encoder); > > > > + power_domain = > > > > intel_display_port_aux_power_domain(intel_encoder); > > > > intel_display_power_get(dev_priv, power_domain); > > > > > > > > if (long_hpd) { > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > > > b/drivers/gpu/drm/i915/intel_drv.h > > > > index a68b6cd..7d11aa0 100644 > > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > > @@ -1188,6 +1188,8 @@ 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); > > > > void intel_modeset_preclose(struct drm_device *dev, struct drm_file > > > > *file); > > > > -- > > Ville Syrjälä > > Intel OTC
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d0fec07..c2578d9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5143,6 +5143,23 @@ 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; + default: + WARN_ON_ONCE(1); + return POWER_DOMAIN_AUX_A; + } +} + #define for_each_power_domain(domain, mask) \ for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \ if ((1 << (domain)) & (mask)) @@ -5174,6 +5191,29 @@ 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_device *dev = 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)); + case INTEL_OUTPUT_DISPLAYPORT: + 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: + WARN_ON_ONCE(1); + return POWER_DOMAIN_AUX_A; + } +} + static unsigned long get_crtc_power_domains(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4655af0..3978540 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -277,7 +277,7 @@ static void pps_lock(struct intel_dp *intel_dp) * See vlv_power_sequencer_reset() why we need * a power domain reference here. */ - power_domain = intel_display_port_power_domain(encoder); + power_domain = intel_display_port_aux_power_domain(encoder); intel_display_power_get(dev_priv, power_domain); mutex_lock(&dev_priv->pps_mutex); @@ -293,7 +293,7 @@ static void pps_unlock(struct intel_dp *intel_dp) mutex_unlock(&dev_priv->pps_mutex); - power_domain = intel_display_port_power_domain(encoder); + power_domain = intel_display_port_aux_power_domain(encoder); intel_display_power_put(dev_priv, power_domain); } @@ -816,8 +816,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, intel_dp_check_edp(intel_dp); - intel_aux_display_runtime_get(dev_priv); - /* Try to wait for any previous AUX channel activity */ for (try = 0; try < 3; try++) { status = I915_READ_NOTRACE(ch_ctl); @@ -926,7 +924,6 @@ done: ret = recv_bytes; out: pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE); - intel_aux_display_runtime_put(dev_priv); if (vdd) edp_panel_vdd_off(intel_dp, false); @@ -1784,7 +1781,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_power_domain(intel_encoder); + power_domain = intel_display_port_aux_power_domain(intel_encoder); intel_display_power_get(dev_priv, power_domain); DRM_DEBUG_KMS("Turning eDP port %c VDD on\n", @@ -1874,7 +1871,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp) if ((pp & POWER_TARGET_ON) == 0) intel_dp->last_power_cycle = jiffies; - power_domain = intel_display_port_power_domain(intel_encoder); + power_domain = intel_display_port_aux_power_domain(intel_encoder); intel_display_power_put(dev_priv, power_domain); } @@ -2025,7 +2022,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_power_domain(intel_encoder); + power_domain = intel_display_port_aux_power_domain(intel_encoder); intel_display_power_put(dev_priv, power_domain); } @@ -4765,26 +4762,6 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) intel_dp->has_audio = false; } -static enum intel_display_power_domain -intel_dp_power_get(struct intel_dp *dp) -{ - struct intel_encoder *encoder = &dp_to_dig_port(dp)->base; - enum intel_display_power_domain power_domain; - - power_domain = intel_display_port_power_domain(encoder); - intel_display_power_get(to_i915(encoder->base.dev), power_domain); - - return power_domain; -} - -static void -intel_dp_power_put(struct intel_dp *dp, - enum intel_display_power_domain power_domain) -{ - struct intel_encoder *encoder = &dp_to_dig_port(dp)->base; - intel_display_power_put(to_i915(encoder->base.dev), power_domain); -} - static enum drm_connector_status intel_dp_detect(struct drm_connector *connector, bool force) { @@ -4808,7 +4785,8 @@ intel_dp_detect(struct drm_connector *connector, bool force) return connector_status_disconnected; } - power_domain = intel_dp_power_get(intel_dp); + power_domain = intel_display_port_aux_power_domain(intel_encoder); + intel_display_power_get(to_i915(dev), power_domain); /* Can't disconnect eDP, but you can close the lid... */ if (is_edp(intel_dp)) @@ -4853,7 +4831,7 @@ intel_dp_detect(struct drm_connector *connector, bool force) } out: - intel_dp_power_put(intel_dp, power_domain); + intel_display_power_put(to_i915(dev), power_domain); return status; } @@ -4862,6 +4840,7 @@ 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", @@ -4871,11 +4850,12 @@ intel_dp_force(struct drm_connector *connector) if (connector->status != connector_status_connected) return; - power_domain = intel_dp_power_get(intel_dp); + power_domain = intel_display_port_aux_power_domain(intel_encoder); + intel_display_power_get(dev_priv, power_domain); intel_dp_set_edid(intel_dp); - intel_dp_power_put(intel_dp, power_domain); + intel_display_power_put(dev_priv, power_domain); if (intel_encoder->type != INTEL_OUTPUT_EDP) intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; @@ -5091,7 +5071,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_power_domain(&intel_dig_port->base); + power_domain = intel_display_port_aux_power_domain(&intel_dig_port->base); intel_display_power_get(dev_priv, power_domain); edp_panel_vdd_schedule_off(intel_dp); @@ -5172,7 +5152,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) port_name(intel_dig_port->port), long_hpd ? "long" : "short"); - power_domain = intel_display_port_power_domain(intel_encoder); + power_domain = intel_display_port_aux_power_domain(intel_encoder); intel_display_power_get(dev_priv, power_domain); if (long_hpd) { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a68b6cd..7d11aa0 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1188,6 +1188,8 @@ 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); void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file);