Em Qui, 2016-02-25 às 14:51 +0200, ville.syrjala at linux.intel.com escreveu: > From: Ville Syrjälä <ville.syrjala at linux.intel.com> > > To save a bit of power, let's try to turn off the TMDS output buffers > in DP++ adaptors when we're not driving the port. > > v2: Let's not forget DDI, toss in a debug message while at it > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com> > --- >  drivers/gpu/drm/i915/intel_ddi.c  | 12 ++++++++++++ >  drivers/gpu/drm/i915/intel_drv.h  |  2 ++ >  drivers/gpu/drm/i915/intel_hdmi.c | 31 > +++++++++++++++++++++++++++++-- >  3 files changed, 43 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 21a9b83f3bfc..458c41788b80 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2301,6 +2301,12 @@ static void intel_ddi_pre_enable(struct > intel_encoder *intel_encoder) >  enum port port = intel_ddi_get_encoder_port(intel_encoder); >  int type = intel_encoder->type; >  > + if (type == INTEL_OUTPUT_HDMI) { > + struct intel_hdmi *intel_hdmi = > enc_to_intel_hdmi(encoder); > + > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, > true); > + } > + >  intel_prepare_ddi_buffer(intel_encoder); >  >  if (type == INTEL_OUTPUT_EDP) { > @@ -2367,6 +2373,12 @@ static void intel_ddi_post_disable(struct > intel_encoder *intel_encoder) >  DPLL_CTRL2_DDI_CLK_OFF(port) > )); >  else if (INTEL_INFO(dev)->gen < 9) >  I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE); > + > + if (type == INTEL_OUTPUT_HDMI) { > + struct intel_hdmi *intel_hdmi = > enc_to_intel_hdmi(encoder); > + > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, > false); > + } >  } >  >  static void intel_enable_ddi(struct intel_encoder *intel_encoder) > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 944eacbfb6a9..2e4fa4337c59 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -706,6 +706,7 @@ struct intel_hdmi { >  int ddc_bus; >  struct { >  int max_tmds_clock; > + bool tmds_output_control; >  } dp_dual_mode; >  bool limited_color_range; >  bool color_range_auto; > @@ -1355,6 +1356,7 @@ void intel_hdmi_init_connector(struct > intel_digital_port *intel_dig_port, >  struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder); >  bool intel_hdmi_compute_config(struct intel_encoder *encoder, >         struct intel_crtc_state > *pipe_config); > +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, > bool enable); >  >  >  /* intel_lvds.c */ > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 1e8cfdb18dc7..4b528a669f8e 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -837,6 +837,20 @@ static void hsw_set_infoframes(struct > drm_encoder *encoder, >  intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode); >  } >  > +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, > bool enable) > +{ > + if (hdmi->dp_dual_mode.tmds_output_control) {
Or you could just do an early return here and save an indentation level :) > + struct drm_i915_private *dev_priv = > to_i915(intel_hdmi_to_dev(hdmi)); > + struct i2c_adapter *adapter = > + intel_gmbus_get_adapter(dev_priv, hdmi- > >ddc_bus); > + > + DRM_DEBUG_KMS("%s DP dual mode adaptor TMDS > output\n", > + Â Â Â Â Â Â enable ? "Enabling" : "Disabling"); > + > + drm_dp_dual_mode_set_tmds_output(adapter, enable); > + } > +} > + > Â static void intel_hdmi_prepare(struct intel_encoder *encoder) > Â { > Â struct drm_device *dev = encoder->base.dev; > @@ -846,6 +860,8 @@ static void intel_hdmi_prepare(struct > intel_encoder *encoder) > Â const struct drm_display_mode *adjusted_mode = &crtc- > >config->base.adjusted_mode; > Â u32 hdmi_val; > Â > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); > + > Â hdmi_val = SDVO_ENCODING_HDMI; > Â if (!HAS_PCH_SPLIT(dev) && crtc->config- > >limited_color_range) > Â hdmi_val |= HDMI_COLOR_RANGE_16_235; > @@ -1144,6 +1160,8 @@ static void intel_disable_hdmi(struct > intel_encoder *encoder) > Â } > Â > Â intel_hdmi->set_infoframes(&encoder->base, false, NULL); > + > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); > Â } > Â > Â static void g4x_disable_hdmi(struct intel_encoder *encoder) > @@ -1369,6 +1387,7 @@ intel_hdmi_unset_edid(struct drm_connector > *connector) > Â intel_hdmi->rgb_quant_range_selectable = false; > Â > Â intel_hdmi->dp_dual_mode.max_tmds_clock = 0; > + intel_hdmi->dp_dual_mode.tmds_output_control = false; > Â > Â kfree(to_intel_connector(connector)->detect_edid); > Â to_intel_connector(connector)->detect_edid = NULL; > @@ -1392,15 +1411,23 @@ intel_hdmi_dp_dual_mode_detect(struct > drm_connector *connector) > Â Â */ > Â if (type == DRM_DP_DUAL_MODE_TYPE2_DVI || > Â Â Â Â Â type == DRM_DP_DUAL_MODE_TYPE2_HDMI) { > + bool tmds_enabled; > + > Â hdmi->dp_dual_mode.max_tmds_clock = > Â drm_dp_dual_mode_max_tmds_clock(adapter); > + > + hdmi->dp_dual_mode.tmds_output_control = > + drm_dp_dual_mode_get_tmds_output(adapter, > &tmds_enabled) == 0 && > + drm_dp_dual_mode_set_tmds_output(adapter, > tmds_enabled) == 0; > Â } else { > Â hdmi->dp_dual_mode.max_tmds_clock = 165000; > + hdmi->dp_dual_mode.tmds_output_control = false; While type 1 adaptors are not required to implement the register, what if they do? We could potentially keep TMDS disabled forever. Maybe my suggestions on p1 would help a little bit here? Anyway, what we have here is already better than the previous state, so if no rebase is required: Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com> > Â } > Â > - DRM_DEBUG_KMS("DP dual mode adaptor (%s) detected (max TMDS > clock: %d kHz)\n", > + DRM_DEBUG_KMS("DP dual mode adaptor (%s) detected (max TMDS > clock: %d kHz, TMDS OE# control: %s)\n", > Â Â Â Â Â Â Â drm_dp_get_dual_mode_type_name(type), > - Â Â Â Â Â Â hdmi->dp_dual_mode.max_tmds_clock); > + Â Â Â Â Â Â hdmi->dp_dual_mode.max_tmds_clock, > + Â Â Â Â Â Â yesno(hdmi- > >dp_dual_mode.tmds_output_control)); > Â } > Â > Â static bool