[PATCH 0/2] Changes in HDMI detection

[PATCH 0/2] Changes in HDMI detection

[Shashank Sharma]

This patch set adds 2 patches:
- First patch optimizes the HDMI detection retry loop
- Second patch restricts the use of live status register for HDMI detection
  to certain platforms. This is to fix a regression introduced by a previos
  patch.

Shashank Sharma (2):
  drm/i915: Optimize the live status retry logic
  drm/i915: Restrict usage of live status check

 drivers/gpu/drm/i915/intel_hdmi.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915: Optimize the live status retry logic

[Shashank Sharma]

There is an extra 'if (try)' check, which can be avoided by
changing the logic slightly, keeping the delay as same(80ms)

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e2dab48..b523a2f 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1397,24 +1397,21 @@ 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;
+	bool live_status = true;
+	unsigned int try = 8;
 
 	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));
+	while (!live_status && try--) {
+		msleep(10);
 		live_status = intel_digital_port_connected(dev_priv,
-				hdmi_to_dig_port(intel_hdmi));
+			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)) {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Shashank Sharma]

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));
+		}
 	}
-
 	intel_hdmi_unset_edid(connector);
 
 	if (intel_hdmi_set_edid(connector, live_status)) {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Optimize the live status retry logic

[Ville Syrjälä]

On Fri, Mar 11, 2016 at 11:50:59PM +0530, Shashank Sharma wrote:
> There is an extra 'if (try)' check, which can be avoided by
> changing the logic slightly, keeping the delay as same(80ms)
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index e2dab48..b523a2f 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1397,24 +1397,21 @@ 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;
> +	bool live_status = true;
> +	unsigned int try = 8;
>  
>  	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));
> +	while (!live_status && try--) {
> +		msleep(10);
>  		live_status = intel_digital_port_connected(dev_priv,
> -				hdmi_to_dig_port(intel_hdmi));
> +			hdmi_to_dig_port(intel_hdmi));
>  	}

I still don't see the point in "optimizing" this and making it harder
to figure out how many times it's doing its thing. It's calling msleep()
and doing i2c, so any performance considerations are clearly out the
window.

>  
> -	if (!live_status)
> -		DRM_DEBUG_KMS("Live status not up!");
> -
>  	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

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

[Ville Syrjälä]

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

Re: [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@linux.intel.com] 
Sent: Saturday, March 12, 2016 12:54 AM
To: Sharma, Shashank
Cc: intel-gfx@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@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

✗ Fi.CI.BAT: failure for Changes in HDMI detection

[Patchwork]

== Series Details ==

Series: Changes in HDMI detection
URL   : https://patchwork.freedesktop.org/series/4378/
State : failure

== Summary ==

Series 4378v1 Changes in HDMI detection
http://patchwork.freedesktop.org/api/1.0/series/4378/revisions/1/mbox/

Test core_prop_blob:
        Subgroup basic:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_ctx_param_basic:
        Subgroup invalid-param-get:
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup invalid-size-set:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_exec_basic:
        Subgroup gtt-bsd1:
                incomplete -> SKIP       (bsw-nuc-2)
        Subgroup readonly-bsd:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_flink_basic:
        Subgroup flink-lifetime:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_mmap_gtt:
        Subgroup basic-small-copy-xy:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_ringfill:
        Subgroup basic-default-hang:
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup basic-default-s3:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test gem_storedw_loop:
        Subgroup basic-bsd1:
                incomplete -> SKIP       (bsw-nuc-2)
Test gem_sync:
        Subgroup basic-blt:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_tiled_blits:
        Subgroup basic:
                incomplete -> PASS       (bsw-nuc-2)
Test kms_addfb_basic:
        Subgroup basic:
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup basic-x-tiled:
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup bo-too-small-due-to-tiling:
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup clobberred-modifier:
                incomplete -> PASS       (bsw-nuc-2)
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                pass       -> SKIP       (ivb-t430s)
        Subgroup force-load-detect:
                pass       -> SKIP       (snb-x220t)
Test kms_pipe_crc_basic:
        Subgroup bad-pipe:
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup nonblocking-crc-pipe-a:
                incomplete -> SKIP       (bsw-nuc-2)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                incomplete -> SKIP       (bsw-nuc-2)
        Subgroup nonblocking-crc-pipe-b:
                incomplete -> SKIP       (bsw-nuc-2)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (snb-x220t)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (hsw-gt2)
Test kms_sink_crc_basic:
                incomplete -> SKIP       (bsw-nuc-2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-fail -> FAIL       (snb-x220t)
                pass       -> DMESG-WARN (snb-dellxps)
                incomplete -> PASS       (bsw-nuc-2)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (snb-x220t)
                dmesg-warn -> PASS       (snb-dellxps)
                pass       -> DMESG-WARN (hsw-gt2)
Test prime_self_import:
        Subgroup basic-llseek-size:
                incomplete -> PASS       (bsw-nuc-2)

bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:194  pass:173  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:194  pass:156  dwarn:1   dfail:0   fail:0   skip:37 
hsw-brixbox      total:194  pass:170  dwarn:2   dfail:0   fail:0   skip:22 
hsw-gt2          total:194  pass:175  dwarn:2   dfail:0   fail:0   skip:17 
ivb-t430s        total:194  pass:168  dwarn:0   dfail:0   fail:0   skip:26 
skl-i5k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:194  pass:183  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:194  pass:159  dwarn:1   dfail:0   fail:0   skip:34 
snb-x220t        total:194  pass:157  dwarn:2   dfail:0   fail:1   skip:34 

Results at /archive/results/CI_IGT_test/Patchwork_1584/

571147d0be8b04cbbe99db761e82ef105c6f82ad drm-intel-nightly: 2016y-03m-11d-13h-31m-03s UTC integration manifest
10cfd562a7eef0090ee3ea6191a40bd776c45300 drm/i915: Restrict usage of live status check
3319e79ffc529c5140db03afec400dadbe18cb93 drm/i915: Optimize the live status retry logic

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Fri, 11 Mar 2016, Shashank Sharma <shashank.sharma@intel.com> wrote:
> [ text/plain ]
> 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

This is in v4.4, which means the fix will need to be backported too, so
please send a patch fixing the bug *first*, instead of depending on an
optimization patch.

BR,
Jani.


>
> 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));
> +		}
>  	}
> -
>  	intel_hdmi_unset_edid(connector);
>  
>  	if (intel_hdmi_set_edid(connector, live_status)) {

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Sharma, Shashank]

+intel-gfx

Regards
Shashank
-----Original Message-----
From: Sharma, Shashank 
Sent: Thursday, March 17, 2016 9:37 AM
To: Jindal, Sonika; Vetter, Daniel; ville.syrjala@linux.intel.com; jani.nikula@linux.intel.com
Cc: Sharma, Shashank
Subject: [PATCH] drm/i915: Restrict usage of live status check

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: 237ed86 "drm/i915: Check live status"
	Author: Sonika Jindal <sonika.jindal@intel.com>
	Date:   Tue Sep 15 09:44:20 2015 +0530

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e2dab48..d24d18a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1397,7 +1397,8 @@ 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;
+	struct drm_device *dev = connector->dev;
+	bool live_status = true;
 	unsigned int try;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
@@ -1405,16 +1406,21 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 
 	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,
+	/*
+	* Live status check for HDMI detection is not very
+	* reliable on older platforms. So insist the live
+	* status check for EDID read from VLV onwards.
+	*/
+	if (INTEL_INFO(dev)->gen >= 7 && !IS_IVYBRIDGE(dev)) {
+		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));
+		}
+		DRM_DEBUG_KMS("Live status %s\n", live_status ? "up" : "down");
 	}
 
-	if (!live_status)
-		DRM_DEBUG_KMS("Live status not up!");
-
 	intel_hdmi_unset_edid(connector);
 
 	if (intel_hdmi_set_edid(connector, live_status)) {
--
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Thu, 17 Mar 2016, "Sharma, Shashank" <shashank.sharma@intel.com> wrote:
> +intel-gfx

Please repost the patch to intel-gfx, just adding cc: makes the patch
painful to review and apply, and CI won't pick it up.

BR,
Jani.

>
> Regards
> Shashank
> -----Original Message-----
> From: Sharma, Shashank 
> Sent: Thursday, March 17, 2016 9:37 AM
> To: Jindal, Sonika; Vetter, Daniel; ville.syrjala@linux.intel.com; jani.nikula@linux.intel.com
> Cc: Sharma, Shashank
> Subject: [PATCH] drm/i915: Restrict usage of live status check
>
> 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: 237ed86 "drm/i915: Check live status"
> 	Author: Sonika Jindal <sonika.jindal@intel.com>
> 	Date:   Tue Sep 15 09:44:20 2015 +0530
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index e2dab48..d24d18a 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1397,7 +1397,8 @@ 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;
> +	struct drm_device *dev = connector->dev;
> +	bool live_status = true;
>  	unsigned int try;
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> @@ -1405,16 +1406,21 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  
>  	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,
> +	/*
> +	* Live status check for HDMI detection is not very
> +	* reliable on older platforms. So insist the live
> +	* status check for EDID read from VLV onwards.
> +	*/
> +	if (INTEL_INFO(dev)->gen >= 7 && !IS_IVYBRIDGE(dev)) {
> +		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));
> +		}
> +		DRM_DEBUG_KMS("Live status %s\n", live_status ? "up" : "down");
>  	}
>  
> -	if (!live_status)
> -		DRM_DEBUG_KMS("Live status not up!");
> -
>  	intel_hdmi_unset_edid(connector);
>  
>  	if (intel_hdmi_set_edid(connector, live_status)) {
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx