public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/i915: Restrict usage of live status check
@ 2016-03-10 13:16 Shashank Sharma
  2016-03-10 15:36 ` ✗ Fi.CI.BAT: failure for drm/i915: Restrict usage of live status check (rev2) Patchwork
  2016-03-11  8:48 ` [PATCH v2] drm/i915: Restrict usage of live status check Jani Nikula
  0 siblings, 2 replies; 7+ messages in thread
From: Shashank Sharma @ 2016-03-10 13:16 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, sonika.jindal, gary.c.wang,
	shobhit.kumar, daniel.vetter

This patch does the following:
- 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
  doesn't work reliably on some older devices. So limit the
  live_status check for HDMI detection, only for platforms
  from gen7 onwards.
- Cleans up the retry logic.
  There is an extra 'if (try)' check, which can be avoided by
  changing the logic slightly, keeping the delay as same(80ms)

V2: Ville: Skipped live_status check for IVB

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

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e2dab48..b1e67de 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1397,24 +1397,32 @@ 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;
+	struct drm_device *dev = dev_priv->dev;
+	bool live_status = true;
+	unsigned int try = 9;
 
 	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);
+	/*
+	 * When tested with specific set of monitors + cables,
+	 * live status behavior is not very consistent for older
+	 * platforms. So add live status check from VLV
+	 * onwards.
+	 */
+	if (INTEL_INFO(dev)->gen >= 7 && !IS_IVYBRIDGE(dev)) {
+retry_ls:
 		live_status = intel_digital_port_connected(dev_priv,
 				hdmi_to_dig_port(intel_hdmi));
+		if (!live_status && --try) {
+			msleep(10);
+			goto retry_ls;
+		}
+		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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915: Restrict usage of live status check (rev2)
  2016-03-10 13:16 [PATCH v2] drm/i915: Restrict usage of live status check Shashank Sharma
@ 2016-03-10 15:36 ` Patchwork
  2016-03-11  8:48 ` [PATCH v2] drm/i915: Restrict usage of live status check Jani Nikula
  1 sibling, 0 replies; 7+ messages in thread
From: Patchwork @ 2016-03-10 15:36 UTC (permalink / raw)
  To: Shashank Sharma; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Restrict usage of live status check (rev2)
URL   : https://patchwork.freedesktop.org/series/4297/
State : failure

== Summary ==

Series 4297v2 drm/i915: Restrict usage of live status check
http://patchwork.freedesktop.org/api/1.0/series/4297/revisions/2/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (bdw-ultra)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (hsw-gt2)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-warn -> PASS       (hsw-brixbox)
        Subgroup basic-rte:
                dmesg-warn -> PASS       (bsw-nuc-2)

bdw-nuci7        total:193  pass:181  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:193  pass:172  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:193  pass:155  dwarn:1   dfail:0   fail:0   skip:37 
hsw-brixbox      total:193  pass:171  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:193  pass:175  dwarn:1   dfail:0   fail:0   skip:17 
ivb-t430s        total:193  pass:168  dwarn:0   dfail:0   fail:0   skip:25 
skl-i5k-2        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:23 
skl-i7k-2        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:161  pass:152  dwarn:0   dfail:0   fail:0   skip:9  
snb-x220t        total:193  pass:158  dwarn:1   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1562/

63461ba49f1bc63fe9dcccdbbaa3a5049c57ae37 drm-intel-nightly: 2016y-03m-10d-13h-21m-51s UTC integration manifest
15df67ec813807692bf78e13462929f20ae491d0 drm/i915: Restrict usage of live status check

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] drm/i915: Restrict usage of live status check
  2016-03-10 13:16 [PATCH v2] drm/i915: Restrict usage of live status check Shashank Sharma
  2016-03-10 15:36 ` ✗ Fi.CI.BAT: failure for drm/i915: Restrict usage of live status check (rev2) Patchwork
@ 2016-03-11  8:48 ` Jani Nikula
  1 sibling, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2016-03-11  8:48 UTC (permalink / raw)
  To: Shashank Sharma, intel-gfx, ville.syrjala, sonika.jindal,
	gary.c.wang, shobhit.kumar, daniel.vetter

On Thu, 10 Mar 2016, Shashank Sharma <shashank.sharma@intel.com> wrote:
> This patch does the following:
> - 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
>   doesn't work reliably on some older devices. So limit the
>   live_status check for HDMI detection, only for platforms
>   from gen7 onwards.
> - Cleans up the retry logic.
>   There is an extra 'if (try)' check, which can be avoided by
>   changing the logic slightly, keeping the delay as same(80ms)

The commit message needs a reference to the commit that broke things.

BR,
Jani.

>
> V2: Ville: Skipped live_status check for IVB
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index e2dab48..b1e67de 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1397,24 +1397,32 @@ 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;
> +	struct drm_device *dev = dev_priv->dev;
> +	bool live_status = true;
> +	unsigned int try = 9;
>  
>  	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);
> +	/*
> +	 * When tested with specific set of monitors + cables,
> +	 * live status behavior is not very consistent for older
> +	 * platforms. So add live status check from VLV
> +	 * onwards.
> +	 */
> +	if (INTEL_INFO(dev)->gen >= 7 && !IS_IVYBRIDGE(dev)) {
> +retry_ls:
>  		live_status = intel_digital_port_connected(dev_priv,
>  				hdmi_to_dig_port(intel_hdmi));
> +		if (!live_status && --try) {
> +			msleep(10);
> +			goto retry_ls;
> +		}
> +		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)) {

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] drm/i915: Restrict usage of live status check
  2016-03-17  9:25 [PATCH] " Jani Nikula
@ 2016-04-20 12:12 ` Shashank Sharma
  2016-04-20 14:48   ` Ville Syrjälä
  0 siblings, 1 reply; 7+ messages in thread
From: Shashank Sharma @ 2016-04-20 12:12 UTC (permalink / raw)
  To: jani.nikula, intel-gfx, ville.syrjala; +Cc: joseph.salisbury, daniel.vetter

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 regression added by patch:
	'commit 237ed86c693d ("drm/i915: Check live status before reading edid")'

===
V2
===
Ville:
- Add live_status =  true in the end of delay loop.

Jani:
- Initialize bool live_status with false;
- Change regression reference in the commit message.
- Use dev_priv instead of dev
- Move declaration of try at the loop

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

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index b199ede..98f5ae0 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1398,22 +1398,39 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 	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,
+	/*
+	* 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_priv)->gen >= 7 && !IS_IVYBRIDGE(dev_priv)) {
+		unsigned int try;
+
+		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!");
+	if (!live_status) {
+		/*
+		* TODO: Replace this with some magic code.
+		* Even after giving enough delay, live status is not up.
+		* Lets read EDID and decide if HDMI is available. We
+		* have to do this not to break old platforms, but ideally
+		* should read EDID only when live_status reg allows us.
+		*/
+		DRM_DEBUG_KMS("Live status down, making it up\n");
+		live_status = true;
+	}
 
 	intel_hdmi_unset_edid(connector);
 
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] drm/i915: Restrict usage of live status check
  2016-04-20 12:12 ` [PATCH v2] " Shashank Sharma
@ 2016-04-20 14:48   ` Ville Syrjälä
  2016-04-21  3:55     ` Sharma, Shashank
  0 siblings, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2016-04-20 14:48 UTC (permalink / raw)
  To: Shashank Sharma; +Cc: intel-gfx, joseph.salisbury, daniel.vetter

On Wed, Apr 20, 2016 at 05:42:51PM +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 regression added by patch:
> 	'commit 237ed86c693d ("drm/i915: Check live status before reading edid")'
> 
> ===
> V2
> ===
> Ville:
> - Add live_status =  true in the end of delay loop.
> 
> Jani:
> - Initialize bool live_status with false;
> - Change regression reference in the commit message.
> - Use dev_priv instead of dev
> - Move declaration of try at the loop
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index b199ede..98f5ae0 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1398,22 +1398,39 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  	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,
> +	/*
> +	* 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_priv)->gen >= 7 && !IS_IVYBRIDGE(dev_priv)) {
> +		unsigned int try;
> +
> +		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!");
> +	if (!live_status) {
> +		/*
> +		* TODO: Replace this with some magic code.
> +		* Even after giving enough delay, live status is not up.
> +		* Lets read EDID and decide if HDMI is available. We
> +		* have to do this not to break old platforms, but ideally
> +		* should read EDID only when live_status reg allows us.
> +		*/
> +		DRM_DEBUG_KMS("Live status down, making it up\n");
> +		live_status = true;
> +	}

Hmm. This is still not what I suggested earlier. Was there some problem
with my idea?

>  
>  	intel_hdmi_unset_edid(connector);
>  
> -- 
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] drm/i915: Restrict usage of live status check
  2016-04-20 14:48   ` Ville Syrjälä
@ 2016-04-21  3:55     ` Sharma, Shashank
  2016-04-21  4:45       ` Sharma, Shashank
  0 siblings, 1 reply; 7+ messages in thread
From: Sharma, Shashank @ 2016-04-21  3:55 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, joseph.salisbury, daniel.vetter

On 4/20/2016 8:18 PM, Ville Syrjälä wrote:
> On Wed, Apr 20, 2016 at 05:42:51PM +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 regression added by patch:
>> 	'commit 237ed86c693d ("drm/i915: Check live status before reading edid")'
>>
>> ===
>> V2
>> ===
>> Ville:
>> - Add live_status =  true in the end of delay loop.
>>
>> Jani:
>> - Initialize bool live_status with false;
>> - Change regression reference in the commit message.
>> - Use dev_priv instead of dev
>> - Move declaration of try at the loop
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_hdmi.c | 31 ++++++++++++++++++++++++-------
>>   1 file changed, 24 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index b199ede..98f5ae0 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1398,22 +1398,39 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>>   	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,
>> +	/*
>> +	* 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_priv)->gen >= 7 && !IS_IVYBRIDGE(dev_priv)) {
>> +		unsigned int try;
>> +
>> +		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!");
>> +	if (!live_status) {
>> +		/*
>> +		* TODO: Replace this with some magic code.
>> +		* Even after giving enough delay, live status is not up.
>> +		* Lets read EDID and decide if HDMI is available. We
>> +		* have to do this not to break old platforms, but ideally
>> +		* should read EDID only when live_status reg allows us.
>> +		*/
>> +		DRM_DEBUG_KMS("Live status down, making it up\n");
>> +		live_status = true;
>> +	}
>
> Hmm. This is still not what I suggested earlier. Was there some problem
> with my idea?
>
Not at all, in fact I thought I was doing just as you told. I guess I 
still missed something, please help me to understand what was that.
our last discussion was:

Ville: Sample code:
if (don't trust live status)
live_status = true;

Shashank: Now, my question is, do you want to remove live_status check for
VLV and other platforms too ? or this is good enough ?

Ville:
"No, I'm objecting to changing the entire code when you could just
add two lines. Also my way has the extra benefit that we keep the
live status check mostly working on these presumed "broken" platforms.
So the only difference to the current situation is that we would
still attempt the EDID read even if live_status came out as false,
but thanks to the extra delay from the live status polling we would
hopefully avoid spurious detection results since the EDID read gets 
delayed a bit"

Did you mean to add a kernel command-line flag type stuff for 'don't 
trust live status' or just gen_check was enough ?

Regards
Shashank
>>
>>   	intel_hdmi_unset_edid(connector);
>>
>> --
>> 1.9.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] drm/i915: Restrict usage of live status check
  2016-04-21  3:55     ` Sharma, Shashank
@ 2016-04-21  4:45       ` Sharma, Shashank
  0 siblings, 0 replies; 7+ messages in thread
From: Sharma, Shashank @ 2016-04-21  4:45 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, joseph.salisbury, daniel.vetter

Hi Ville,
Please ignore my last email.

I think I finally got it what you exactly suggested.
Please check another patch sent by me:
	drm/i915: Fake HDMI live status

There are two versions of this patch:
	v1: fake live_status = true for all platforms.
	v2: fake live_status = true only for (< gen7 and IVB platforms).

please suggest which one will you prefer to go as a fix.

Regards
Shashank
On 4/21/2016 9:25 AM, Sharma, Shashank wrote:
> On 4/20/2016 8:18 PM, Ville Syrjälä wrote:
>> On Wed, Apr 20, 2016 at 05:42:51PM +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 regression added by patch:
>>>     'commit 237ed86c693d ("drm/i915: Check live status before reading
>>> edid")'
>>>
>>> ===
>>> V2
>>> ===
>>> Ville:
>>> - Add live_status =  true in the end of delay loop.
>>>
>>> Jani:
>>> - Initialize bool live_status with false;
>>> - Change regression reference in the commit message.
>>> - Use dev_priv instead of dev
>>> - Move declaration of try at the loop
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_hdmi.c | 31 ++++++++++++++++++++++++-------
>>>   1 file changed, 24 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>>> b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index b199ede..98f5ae0 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -1398,22 +1398,39 @@ intel_hdmi_detect(struct drm_connector
>>> *connector, bool force)
>>>       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,
>>> +    /*
>>> +    * 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_priv)->gen >= 7 && !IS_IVYBRIDGE(dev_priv)) {
>>> +        unsigned int try;
>>> +
>>> +        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!");
>>> +    if (!live_status) {
>>> +        /*
>>> +        * TODO: Replace this with some magic code.
>>> +        * Even after giving enough delay, live status is not up.
>>> +        * Lets read EDID and decide if HDMI is available. We
>>> +        * have to do this not to break old platforms, but ideally
>>> +        * should read EDID only when live_status reg allows us.
>>> +        */
>>> +        DRM_DEBUG_KMS("Live status down, making it up\n");
>>> +        live_status = true;
>>> +    }
>>
>> Hmm. This is still not what I suggested earlier. Was there some problem
>> with my idea?
>>
> Not at all, in fact I thought I was doing just as you told. I guess I
> still missed something, please help me to understand what was that.
> our last discussion was:
>
> Ville: Sample code:
> if (don't trust live status)
> live_status = true;
>
> Shashank: Now, my question is, do you want to remove live_status check for
> VLV and other platforms too ? or this is good enough ?
>
> Ville:
> "No, I'm objecting to changing the entire code when you could just
> add two lines. Also my way has the extra benefit that we keep the
> live status check mostly working on these presumed "broken" platforms.
> So the only difference to the current situation is that we would
> still attempt the EDID read even if live_status came out as false,
> but thanks to the extra delay from the live status polling we would
> hopefully avoid spurious detection results since the EDID read gets
> delayed a bit"
>
> Did you mean to add a kernel command-line flag type stuff for 'don't
> trust live status' or just gen_check was enough ?
>
> Regards
> Shashank
>>>
>>>       intel_hdmi_unset_edid(connector);
>>>
>>> --
>>> 1.9.1
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-04-21  4:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-10 13:16 [PATCH v2] drm/i915: Restrict usage of live status check Shashank Sharma
2016-03-10 15:36 ` ✗ Fi.CI.BAT: failure for drm/i915: Restrict usage of live status check (rev2) Patchwork
2016-03-11  8:48 ` [PATCH v2] drm/i915: Restrict usage of live status check Jani Nikula
  -- strict thread matches above, loose matches on Subject: below --
2016-03-17  9:25 [PATCH] " Jani Nikula
2016-04-20 12:12 ` [PATCH v2] " Shashank Sharma
2016-04-20 14:48   ` Ville Syrjälä
2016-04-21  3:55     ` Sharma, Shashank
2016-04-21  4:45       ` Sharma, Shashank

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox