On Thu, Mar 31, 2016 at 07:26:25PM +0000, Zanoni, Paulo R wrote:
Em Qui, 2016-02-25 às 14:51 +0200, ville.syrjala@linux.intel.com escreveu:
From: Ville Syrjälä ville.syrjala@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@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 :)
Indeed. I had the code inlined originally so the extra indentation made sense, and the I just copypasted it to this separate function, and in my haste forgot to flatten it.
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 TMDSoutput\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?
I suppose we could just always do the i2c write and if the adaptor supports it everything is peachy, and if it doesn't, well, then I suppose the write just goes to /dev/null and nothing should happen.
We could potentially keep TMDS disabled forever.
That would be an entirely broken adaptor since it would not work at all with source devices that are not dual mode aware.
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@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