[Intel-gfx] [PATCH] drm: i915: Improve behavior in case of broken HDMI EDID

[Daniel Vetter]

On Tue, Apr 19, 2016 at 02:31:13PM -0300, Ezequiel Garcia wrote:
> Currently, our implementation of drm_connector_funcs.detect is
> based on getting a valid EDID.
> 
> This requirement makes the driver fail to detect connected
> connectors in case of EDID corruption, which in turn prevents
> from falling back to modes provided by builtin or user-provided
> EDIDs.

Imo, this should be fixed in the probe helpers. Something like the below
might make sense:


diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index e714b5a7955f..d3b9dc7535da 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -214,7 +214,10 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		else
 			connector->status = connector_status_disconnected;
 		if (connector->funcs->force)
-			connector->funcs->force(connector);
+		connector->funcs->force(connector);
+	} else if (connector->override_edid){
+		connector->status = connector_status_connected;
+		connector->funcs->force(connector);
 	} else {
 		connector->status = connector->funcs->detect(connector, true);
 	}


It should do what you want it to do, still allow us to override force
state manually and also fix things up for every, not just i915-hdmi. Also,
much smaller patch.

Only downside is that we need acks from other driver maintainers, since
essentially it's a behaviour change. Thus far you had to both inject the
edid and override status if your sink was totally busted. Now just
injecting edid will be enough.

Cheers, Daniel

> 
> Let's fix this by calling drm_probe_ddc in drm_connector_funcs.detect,
> and do the EDID full reading and parsing in
> drm_connector_helper_funcs.get_modes, when it's actually needed.
> 
> This patch allows i915 to take advantage of the DRM_LOAD_EDID_FIRMWARE
> infrastructure.
> 
> Without this patch, any device that fails to provide a valid
> EDID will be reported as disconnected (unless the state is forced)
> and thus the kernel won't allow to use such device with any mode,
> either builtin, user-provided, or the 1024x768 noedid fallback.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel at vanguardiasur.com.ar>
> ---
> This patch supersedes: "drm/i915/hdmi: Fix weak connector detection",
> https://patchwork.freedesktop.org/patch/79098/.
> 
>  drivers/gpu/drm/i915/intel_hdmi.c | 59 ++++++++++++++++++++++++---------------
>  1 file changed, 36 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 616108c4bc3e..aa2f2271394a 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1392,36 +1392,17 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  	enum drm_connector_status status;
>  	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> -	bool live_status = false;
> -	unsigned int try;
> -
> -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> -		      connector->base.id, connector->name);
> +	struct i2c_adapter *adap;
>  
>  	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>  
> -	for (try = 0; !live_status && try < 9; try++) {
> -		if (try)
> -			msleep(10);
> -		live_status = intel_digital_port_connected(dev_priv,
> -				hdmi_to_dig_port(intel_hdmi));
> -	}
> -
> -	if (!live_status)
> -		DRM_DEBUG_KMS("Live status not up!");
> -
> -	intel_hdmi_unset_edid(connector);
> -
> -	if (intel_hdmi_set_edid(connector, live_status)) {
> -		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> -
> -		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> +	adap = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
> +	if (drm_probe_ddc(adap))
>  		status = connector_status_connected;
> -	} else
> +	else
>  		status = connector_status_disconnected;
>  
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> -
>  	return status;
>  }
>  
> @@ -1442,10 +1423,42 @@ intel_hdmi_force(struct drm_connector *connector)
>  	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
>  }
>  
> +static void intel_hdmi_detect_edid(struct drm_connector *connector)
> +{
> +	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +	bool live_status = false;
> +	unsigned int try;
> +
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> +		      connector->base.id, connector->name);
> +
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
> +
> +	for (try = 0; !live_status && try < 9; try++) {
> +		if (try)
> +			msleep(10);
> +		live_status = intel_digital_port_connected(dev_priv,
> +				hdmi_to_dig_port(intel_hdmi));
> +	}
> +
> +	if (!live_status)
> +		DRM_DEBUG_KMS("Live status not up!");
> +
> +	intel_hdmi_unset_edid(connector);
> +	if (intel_hdmi_set_edid(connector, live_status))
> +		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> +
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> +}
> +
>  static int intel_hdmi_get_modes(struct drm_connector *connector)
>  {
>  	struct edid *edid;
>  
> +	if (!to_intel_connector(connector)->detect_edid)
> +		intel_hdmi_detect_edid(connector);
> +
>  	edid = to_intel_connector(connector)->detect_edid;
>  	if (edid == NULL)
>  		return 0;
> -- 
> 2.7.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch