Re: [PATCH 5/5] drm/i915: don't init DP or HDMI when not supported by DDI port

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Sep 12, 2013 at 05:12:18PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> There's no reason to init a DP connector if the encoder just supports
> HDMI: we'll just waste hundreds and hundreds of cycles trying to do DP
> AUX transactions to detect if there's something there. Same goes for a
> DP connector that doesn't support HDMI, but I'm not sure these
> actually exist.
> 
> v2: - Use bit fields
>     - Remove useless identation level
>     - Replace DRM_ERROR with DRM_DEBUG_KMS
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> (v1)

Slurped in the entire series, thanks for the patches.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  4 ++++
>  drivers/gpu/drm/i915/intel_bios.c | 13 ++++++++++++-
>  drivers/gpu/drm/i915/intel_ddi.c  | 20 ++++++++++++++++----
>  3 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f159df0..dd94731 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1054,6 +1054,10 @@ enum modeset_restore {
>  
>  struct ddi_vbt_port_info {
>  	uint8_t hdmi_level_shift;
> +
> +	uint8_t supports_dvi:1;
> +	uint8_t supports_hdmi:1;
> +	uint8_t supports_dp:1;
>  };
>  
>  struct intel_vbt_data {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 7ce1c3c..0492b6f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -632,6 +632,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  	is_hdmi = is_dvi && (child->common.device_type & (1 << 11)) == 0;
>  	is_edp = is_dp && (child->common.device_type & (1 << 12));
>  
> +	info->supports_dvi = is_dvi;
> +	info->supports_hdmi = is_hdmi;
> +	info->supports_dp = is_dp;
> +
>  	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n",
>  		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt);
>  
> @@ -792,8 +796,15 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
>  	DRM_DEBUG_KMS("Set default to SSC at %dMHz\n", dev_priv->vbt.lvds_ssc_freq);
>  
>  	for (port = PORT_A; port < I915_MAX_PORTS; port++) {
> +		struct ddi_vbt_port_info *info =
> +			&dev_priv->vbt.ddi_port_info[port];
> +
>  		/* Recommended BSpec default: 800mV 0dB. */
> -		dev_priv->vbt.ddi_port_info[port].hdmi_level_shift = 6;
> +		info->hdmi_level_shift = 6;
> +
> +		info->supports_dvi = (port != PORT_A && port != PORT_E);
> +		info->supports_hdmi = info->supports_dvi;
> +		info->supports_dp = (port != PORT_E);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index a9887eb..488f4a4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1325,6 +1325,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  	struct drm_encoder *encoder;
>  	struct intel_connector *hdmi_connector = NULL;
>  	struct intel_connector *dp_connector = NULL;
> +	bool init_hdmi, init_dp;
> +
> +	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
> +		     dev_priv->vbt.ddi_port_info[port].supports_hdmi);
> +	init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
> +	if (!init_dp && !init_hdmi) {
> +		DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible\n",
> +			      port_name(port));
> +		init_hdmi = true;
> +		init_dp = true;
> +	}
>  
>  	intel_dig_port = kzalloc(sizeof(struct intel_digital_port), GFP_KERNEL);
>  	if (!intel_dig_port)
> @@ -1362,19 +1373,20 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  	intel_encoder->cloneable = false;
>  	intel_encoder->hot_plug = intel_ddi_hot_plug;
>  
> -	if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
> +	if (init_dp && !intel_dp_init_connector(intel_dig_port, dp_connector)) {
>  		drm_encoder_cleanup(encoder);
>  		kfree(intel_dig_port);
>  		kfree(dp_connector);
>  		return;
>  	}
>  
> -	if (intel_encoder->type != INTEL_OUTPUT_EDP) {
> +	/* In theory we don't need the encoder->type check, but leave it just in
> +	 * case we have some really bad VBTs... */
> +	if (intel_encoder->type != INTEL_OUTPUT_EDP && init_hdmi) {
>  		hdmi_connector = kzalloc(sizeof(struct intel_connector),
>  					 GFP_KERNEL);
> -		if (!hdmi_connector) {
> +		if (!hdmi_connector)
>  			return;
> -		}
>  
>  		intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
>  		intel_hdmi_init_connector(intel_dig_port, hdmi_connector);
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch