Re: [PATCH 2/2] drm/i915: Restrict usage of live status check

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Mar 11, 2016 at 11:51:00PM +0530, Shashank Sharma wrote:
> This patch restricts usage of live status check for HDMI detection.
> While testing certain (monitor + cable) combinations with various
> intel  platforms, it seems that live status register is not reliable
> on some older devices. So limit the live_status check from VLV onwards.
> 
> This fixes a regression introduced in:
> commit 237ed86c693d8a8e4db476976aeb30df4deac74b
> Author: Sonika Jindal <sonika.jindal@intel.com>
> Date:   Tue Sep 15 09:44:20 2015 +0530
> drm/i915: Check live status before reading edid
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index b523a2f..ca1fb57 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1397,6 +1397,7 @@ 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);
> +	struct drm_device *dev = connector->dev;
>  	bool live_status = true;
>  	unsigned int try = 8;
>  
> @@ -1404,14 +1405,21 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  		      connector->base.id, connector->name);
>  
>  	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
> -	live_status = intel_digital_port_connected(dev_priv,
> -		hdmi_to_dig_port(intel_hdmi));
> -	while (!live_status && try--) {
> -		msleep(10);
> +
> +	/*
> +	* The live status register doesn't work reliably with certain
> +	* cables/monitors, on old platforms. So restrict the live status
> +	* check to be applied from VLV onwards.
> +	*/
> +	if (INTEL_INFO(dev)->gen >= 7 && !IS_IVYBRIDGE(dev)) {
>  		live_status = intel_digital_port_connected(dev_priv,
>  			hdmi_to_dig_port(intel_hdmi));
> +		while (!live_status && try--) {
> +			msleep(10);
> +			live_status = intel_digital_port_connected(dev_priv,
> +				hdmi_to_dig_port(intel_hdmi));
> +		}
>  	}
> -

I would avoid all that churn and just do 

if (whatever)
	live_status = true;

after we're done with the loop.

That might at least hang on to some of the benefits of the live status
check for these platforms, whether or not the live status really works
or not.

Not that I particularly like this random choice of platforms approach
either.

>  	intel_hdmi_unset_edid(connector);
>  
>  	if (intel_hdmi_set_edid(connector, live_status)) {
> -- 
> 1.9.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx