[Intel-gfx] [PATCH] drm/i915: Reject modeset when the same digital port is used more than once

[Daniel Vetter]

On Mon, May 19, 2014 at 05:19:09PM +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> On pre-HSW we have two encoders per digital port: one HDMI, one DP.
> However they are the same physical port in hardware and we can't enable
> both at the same time. Reject the modeset if the user attempts this.
> 
> So far we've been saved by the fact that we never see both HDMI and DP
> connectors as connected. But if the user decides to force a mode anyway,
> all kinds of funny stuff might happen.
> 
> Unfortunately we don't seem to have any way to inform userspace that
> such configurations are invalid except by returning an error from
> setcrtc. possible_clones only covers real cloning situations, and
> looking at the connector names doesn't work either since we don't
> always register both connectors for the same port. I suppose the
> only way to fix that would be to expose only a single encoder per
> digital port like we do on HSW+ but that would be a fairly large
> undertaking for little gain.
> 
> kms_setmode hits this since it forces modes on non-connected VGA and
> HDMI connectors. Previosuly it just resulted in weirdness such as
> failed link training. With this patch it will now get an error back
> from the kernel and will die with an assert since it thinks that the
> configuration should be fine.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Do we have a bugzilla for this somewhere?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a432348..13add38 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9621,6 +9621,45 @@ static bool check_encoder_cloning(struct intel_crtc *crtc)
>  	return true;
>  }
>  
> +static bool check_digital_port_conflicts(struct drm_device *dev)
> +{
> +	struct intel_connector *connector;
> +	unsigned int used_ports = 0;
> +
> +	/*
> +	 * Walk the connector list instead of the encoder
> +	 * list to detect the problem on ddi platforms
> +	 * where there's just one encoder per digital port.
> +	 */
> +	list_for_each_entry(connector,
> +			    &dev->mode_config.connector_list, base.head) {
> +		struct intel_encoder *encoder = connector->new_encoder;
> +
> +		if (!encoder)
> +			continue;
> +
> +		WARN_ON(!encoder->new_crtc);
> +
> +		switch (encoder->type) {
> +			unsigned int port_mask;
> +		case INTEL_OUTPUT_DISPLAYPORT:
> +		case INTEL_OUTPUT_HDMI:
> +		case INTEL_OUTPUT_EDP:
> +			port_mask = 1 << enc_to_dig_port(&encoder->base)->port;
> +
> +			/* the same port mustn't appear more than once */
> +			if (used_ports & port_mask)
> +				return false;
> +
> +			used_ports |= port_mask;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	return true;
> +}
> +
>  static struct intel_crtc_config *
>  intel_modeset_pipe_config(struct drm_crtc *crtc,
>  			  struct drm_framebuffer *fb,
> @@ -9637,6 +9676,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> +	if (!check_digital_port_conflicts(dev)) {
> +		DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
>  	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
>  	if (!pipe_config)
>  		return ERR_PTR(-ENOMEM);
> -- 
> 1.8.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at 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