Re: [PATCH v2 3/4] drm/i915: Enable/disable TMDS output buffers in DP++ adaptor as needed

["Zanoni, Paulo R" <paulo.r.zanoni@intel.com>]

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
:)

> +		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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx