* [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
* Re: [PATCH] drm/i915: Restrict usage of live status check
@ 2016-03-17 9:25 Jani Nikula
2016-04-20 12:12 ` [PATCH v2] " Shashank Sharma
0 siblings, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2016-03-17 9:25 UTC (permalink / raw)
To: Shashank Sharma, intel-gfx; +Cc: daniel.vetter
On Thu, 17 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: 237ed86 "drm/i915: Check live status"
> Author: Sonika Jindal <sonika.jindal@intel.com>
> Date: Tue Sep 15 09:44:20 2015 +0530
>
Fixes: 237ed86c693d ("drm/i915: Check live status before reading edid")
Cc: stable@vger.kernel.org # v4.4
Bugzilla: ?
> 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;
This is unnecessary.
> + 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)) {
Replace dev with dev_priv. Both work, it's magic, but we prefer using
dev_priv where possible.
Move try variable declaration here.
> + for (try = 0; !live_status && try < 9; try++) {
You'll now ignore live_status on all platforms. (But please do not turn
this into a do-while, keep it as a for loop.)
> + 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");
Consider using onoff(live_status) instead of 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