[Intel-gfx] [PATCH 2/2] drm/i915: Restrict usage of live status check

[Sharma, Shashank]

Right now live status check is helping in many scenarios, including:
- HDMI compliance
- HDMI connector status to be shown to user space
- While running HDCP compliance, we pass a train of hot-plugs/unplugs via analyzers, and without live status
check and HDMI optimization patches, our software state machine is in a soup. 

I don't see any other way of solving all the above problems, without this, and fixing the regression too on the
old devices.  So if you think we should remove this, we should also have some alternative solution for above listed 
problems. Our hotplug handling is not the best way it should be done (both in HW and SW), and we will end
up something like this only. 

We can't say that we are not going to bother about these problem, as we shouldn't focus only on the 
upstream use cases, but Android and Chrome cases also, which are commercial targets, where meeting the
product quality gets even more difficult. 

If you agree, we can discus more about how to make this work, else the only thing I can come up with is we will revert
the HDMI optimization and live status check from upstream, and keep them local on Android branches. 

Regards
Shashank
-----Original Message-----
From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com] 
Sent: Saturday, March 12, 2016 12:54 AM
To: Sharma, Shashank
Cc: intel-gfx at lists.freedesktop.org; Jindal, Sonika; Kumar, Shobhit; Wang, Gary C; Vetter, Daniel
Subject: Re: [PATCH 2/2] drm/i915: Restrict usage of live status check

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 at 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 at 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