* [PATCH 0/2] Live status detection fixes
@ 2016-08-17 8:02 David Weinehall
2016-08-17 8:02 ` [PATCH 1/2] drm/i915: Skip live status if not supported David Weinehall
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: David Weinehall @ 2016-08-17 8:02 UTC (permalink / raw)
To: intel-gfx
With the introduction of the extended timeouts for live status
detection to accomodate for displays that does not immediately
answer to live status requests, we also introduced rather large
overhead whenever we run intel_hdmi_detect() -- the overhead
is multiplied by the number of HDMI-ports.
The first patch here skips the live status check completely for
platforms where the results are unreliable (without the patch
we'd do the check, then ignore the result). The second patch
removes the extended timeouts by default, but offers options
to either use the extended timeouts, or completely disable
live status check.
David Weinehall (2):
drm/i915: Skip live status if not supported
drm/i915: add module param for live_status
drivers/gpu/drm/i915/i915_params.c | 8 ++++++++
drivers/gpu/drm/i915/i915_params.h | 1 +
drivers/gpu/drm/i915/intel_hdmi.c | 36 ++++++++++++++++++++++++++----------
3 files changed, 35 insertions(+), 10 deletions(-)
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] drm/i915: Skip live status if not supported
2016-08-17 8:02 [PATCH 0/2] Live status detection fixes David Weinehall
@ 2016-08-17 8:02 ` David Weinehall
2016-08-17 8:02 ` [PATCH 2/2] drm/i915: add module param for live_status David Weinehall
2016-08-17 8:35 ` ✗ Ro.CI.BAT: failure for Live status detection fixes Patchwork
2 siblings, 0 replies; 8+ messages in thread
From: David Weinehall @ 2016-08-17 8:02 UTC (permalink / raw)
To: intel-gfx
Rather than first attempting to use live status on platforms where
its functionality is spotty, and then forcing it to true no matter
what, we first check whether we're on a supported platform,
*then* try to detect whether there's anything connected or not.
This doesn't improve things for newer platforms, but for older
platforms we save ~80ms / HDMI-port with no display connected
whenever intel_hdmi_detect() is called.
Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
---
drivers/gpu/drm/i915/intel_hdmi.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 4df9f384910c..713c91ce7f70 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1475,6 +1475,14 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
+ /*
+ * Live status register is not reliable on all intel platforms.
+ * So consider live_status only for certain platforms, for
+ * others, read EDID to determine presence of sink.
+ */
+ if (INTEL_INFO(dev_priv)->gen < 7 || IS_IVYBRIDGE(dev_priv))
+ live_status = true;
+
for (try = 0; !live_status && try < 9; try++) {
if (try)
msleep(10);
@@ -1482,16 +1490,8 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
hdmi_to_dig_port(intel_hdmi));
}
- if (!live_status) {
+ if (!live_status)
DRM_DEBUG_KMS("HDMI live status down\n");
- /*
- * Live status register is not reliable on all intel platforms.
- * So consider live_status only for certain platforms, for
- * others, read EDID to determine presence of sink.
- */
- if (INTEL_INFO(dev_priv)->gen < 7 || IS_IVYBRIDGE(dev_priv))
- live_status = true;
- }
intel_hdmi_unset_edid(connector);
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] drm/i915: add module param for live_status
2016-08-17 8:02 [PATCH 0/2] Live status detection fixes David Weinehall
2016-08-17 8:02 ` [PATCH 1/2] drm/i915: Skip live status if not supported David Weinehall
@ 2016-08-17 8:02 ` David Weinehall
2016-08-17 8:08 ` Chris Wilson
2016-08-17 8:35 ` ✗ Ro.CI.BAT: failure for Live status detection fixes Patchwork
2 siblings, 1 reply; 8+ messages in thread
From: David Weinehall @ 2016-08-17 8:02 UTC (permalink / raw)
To: intel-gfx
Since the workaround for buggy displays that do not reply to
live status detect immediately affects a rather limited set of
displays, and since the price paid (almost 100ms per HDMI-port),
we should have that hack disabled by default.
Rather than leaving people with these kinds of broken displays
out in the cold completely, add a module parameter, defaulting
to -1 (live status detection on supported platforms, but without
the extra delays), that allows for re-enabling this hack.
Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
---
drivers/gpu/drm/i915/i915_params.c | 8 ++++++++
drivers/gpu/drm/i915/i915_params.h | 1 +
drivers/gpu/drm/i915/intel_hdmi.c | 20 ++++++++++++++++++--
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 768ad89d9cd4..ad5988b17656 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -62,6 +62,7 @@ struct i915_params i915 __read_mostly = {
.inject_load_failure = 0,
.enable_dpcd_backlight = false,
.enable_gvt = false,
+ .live_status = -1,
};
module_param_named(modeset, i915.modeset, int, 0400);
@@ -233,3 +234,10 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
MODULE_PARM_DESC(enable_gvt,
"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
+
+module_param_named(live_status, i915.live_status, int, 0600);
+MODULE_PARM_DESC(live_status,
+ "Enable live status detection "
+ "(-1=auto [default], "
+ "0=disabled, "
+ "1=enabled with extra delay)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 3a0dd78ddb38..2ff2083936e3 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -52,6 +52,7 @@ struct i915_params {
int mmio_debug;
int edp_vswing;
unsigned int inject_load_failure;
+ int live_status;
/* leave bools at the end to not create holes */
bool enable_hangcheck;
bool fastboot;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 713c91ce7f70..b7d88728335a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1468,6 +1468,7 @@ 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;
+ int attempts = 1;
unsigned int try;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
@@ -1478,12 +1479,27 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
/*
* Live status register is not reliable on all intel platforms.
* So consider live_status only for certain platforms, for
- * others, read EDID to determine presence of sink.
+ * others, read EDID to determine presence of sink, unless
+ * the users explicitly disables live status reads. For users
+ * who have broken displays we offer the option to use
+ * live status with an extra delay.
*/
+ switch (i915.live_status) {
+ case 0:
+ live_status = true;
+ break;
+ case 1:
+ attempts = 9;
+ break;
+ default:
+ case -1:
+ break;
+ }
+
if (INTEL_INFO(dev_priv)->gen < 7 || IS_IVYBRIDGE(dev_priv))
live_status = true;
- for (try = 0; !live_status && try < 9; try++) {
+ for (try = 0; !live_status && try < attempts; try++) {
if (try)
msleep(10);
live_status = intel_digital_port_connected(dev_priv,
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915: add module param for live_status
2016-08-17 8:02 ` [PATCH 2/2] drm/i915: add module param for live_status David Weinehall
@ 2016-08-17 8:08 ` Chris Wilson
2016-08-17 9:25 ` David Weinehall
0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2016-08-17 8:08 UTC (permalink / raw)
To: David Weinehall; +Cc: intel-gfx
On Wed, Aug 17, 2016 at 11:02:32AM +0300, David Weinehall wrote:
> Since the workaround for buggy displays that do not reply to
> live status detect immediately affects a rather limited set of
> displays, and since the price paid (almost 100ms per HDMI-port),
> we should have that hack disabled by default.
>
> Rather than leaving people with these kinds of broken displays
> out in the cold completely, add a module parameter, defaulting
> to -1 (live status detection on supported platforms, but without
> the extra delays), that allows for re-enabling this hack.
No. It is a regression. We revert back to the previous status quo,
and then try to introduce live-status checking in a way that works if at
all possible.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* ✗ Ro.CI.BAT: failure for Live status detection fixes
2016-08-17 8:02 [PATCH 0/2] Live status detection fixes David Weinehall
2016-08-17 8:02 ` [PATCH 1/2] drm/i915: Skip live status if not supported David Weinehall
2016-08-17 8:02 ` [PATCH 2/2] drm/i915: add module param for live_status David Weinehall
@ 2016-08-17 8:35 ` Patchwork
2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2016-08-17 8:35 UTC (permalink / raw)
To: David Weinehall; +Cc: intel-gfx
== Series Details ==
Series: Live status detection fixes
URL : https://patchwork.freedesktop.org/series/11188/
State : failure
== Summary ==
Series 11188v1 Live status detection fixes
http://patchwork.freedesktop.org/api/1.0/series/11188/revisions/1/mbox
Test gem_exec_gttfill:
Subgroup basic:
skip -> PASS (fi-snb-i7-2600)
Test gem_exec_suspend:
Subgroup basic-s3:
dmesg-warn -> PASS (ro-bdw-i7-5600u)
Test kms_cursor_legacy:
Subgroup basic-flip-vs-cursor-legacy:
pass -> FAIL (ro-byt-n2820)
pass -> FAIL (fi-hsw-i7-4770k)
Subgroup basic-flip-vs-cursor-varying-size:
pass -> FAIL (ro-bdw-i5-5250u)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> INCOMPLETE (fi-hsw-i7-4770k)
fi-hsw-i7-4770k total:207 pass:185 dwarn:0 dfail:0 fail:1 skip:20
fi-snb-i7-2600 total:244 pass:202 dwarn:0 dfail:0 fail:0 skip:42
ro-bdw-i5-5250u total:240 pass:218 dwarn:2 dfail:0 fail:2 skip:18
ro-bdw-i7-5557U total:240 pass:220 dwarn:3 dfail:0 fail:0 skip:17
ro-bdw-i7-5600u total:240 pass:207 dwarn:0 dfail:0 fail:1 skip:32
ro-bsw-n3050 total:240 pass:194 dwarn:0 dfail:0 fail:4 skip:42
ro-byt-n2820 total:240 pass:197 dwarn:0 dfail:0 fail:3 skip:40
ro-hsw-i3-4010u total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26
ro-hsw-i7-4770r total:240 pass:185 dwarn:0 dfail:0 fail:0 skip:55
ro-ilk1-i5-650 total:235 pass:174 dwarn:0 dfail:0 fail:1 skip:60
ro-ivb-i7-3770 total:240 pass:205 dwarn:0 dfail:0 fail:0 skip:35
ro-ivb2-i7-3770 total:240 pass:209 dwarn:0 dfail:0 fail:0 skip:31
ro-skl3-i5-6260u total:240 pass:222 dwarn:0 dfail:0 fail:4 skip:14
Results at /archive/results/CI_IGT_test/RO_Patchwork_1896/
a363b72 drm-intel-nightly: 2016y-08m-17d-05h-29m-02s UTC integration manifest
a78e123 drm/i915: add module param for live_status
989ac09 drm/i915: Skip live status if not supported
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915: add module param for live_status
2016-08-17 8:08 ` Chris Wilson
@ 2016-08-17 9:25 ` David Weinehall
2016-08-17 9:35 ` Chris Wilson
0 siblings, 1 reply; 8+ messages in thread
From: David Weinehall @ 2016-08-17 9:25 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Wed, Aug 17, 2016 at 09:08:58AM +0100, Chris Wilson wrote:
> On Wed, Aug 17, 2016 at 11:02:32AM +0300, David Weinehall wrote:
> > Since the workaround for buggy displays that do not reply to
> > live status detect immediately affects a rather limited set of
> > displays, and since the price paid (almost 100ms per HDMI-port),
> > we should have that hack disabled by default.
> >
> > Rather than leaving people with these kinds of broken displays
> > out in the cold completely, add a module parameter, defaulting
> > to -1 (live status detection on supported platforms, but without
> > the extra delays), that allows for re-enabling this hack.
>
> No. It is a regression. We revert back to the previous status quo,
> and then try to introduce live-status checking in a way that works if at
> all possible.
The way I understand it, the only approaches that would allow for
combining live status checking with buggy displays are:
* The current behaviour (unconditionally waiting until we're sure
that even the buggiest displays replies; wastes ~90ms per port
on working setups when there's no display connected)
or
* live status check as in my patch, with the additional delay
configurable
The other option is not to bother with with live status check at all.
It seems to work just fine for anything < gen 7; I'm not sure
if it's necessary for any newer setups either.
Seeing as I'm trying to optimise suspend/resume times, I'm kinda
biased towards any solutions that removes the delays by default.
Whether we remove them by doing the live status check without retries
by default (forcing users with buggy displays to enable the workaround)
or by ripping out the live status check completely isn't really
all that important to me. As long as we can get rid of the current
overhead.
Kind regards, David Weienhall
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915: add module param for live_status
2016-08-17 9:25 ` David Weinehall
@ 2016-08-17 9:35 ` Chris Wilson
2016-08-18 9:56 ` Daniel Vetter
0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2016-08-17 9:35 UTC (permalink / raw)
To: intel-gfx
On Wed, Aug 17, 2016 at 12:25:40PM +0300, David Weinehall wrote:
> On Wed, Aug 17, 2016 at 09:08:58AM +0100, Chris Wilson wrote:
> > On Wed, Aug 17, 2016 at 11:02:32AM +0300, David Weinehall wrote:
> > > Since the workaround for buggy displays that do not reply to
> > > live status detect immediately affects a rather limited set of
> > > displays, and since the price paid (almost 100ms per HDMI-port),
> > > we should have that hack disabled by default.
> > >
> > > Rather than leaving people with these kinds of broken displays
> > > out in the cold completely, add a module parameter, defaulting
> > > to -1 (live status detection on supported platforms, but without
> > > the extra delays), that allows for re-enabling this hack.
> >
> > No. It is a regression. We revert back to the previous status quo,
> > and then try to introduce live-status checking in a way that works if at
> > all possible.
>
> The way I understand it, the only approaches that would allow for
> combining live status checking with buggy displays are:
I haven't seen any convincing analysis as to why live-status works
better than ddc 0x50. Certainly not in the changelogs.
The rule is that even if you fix one system, if you break anything else
in the process and you cannot fix it promptly you revert back to the
previous state and try again.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915: add module param for live_status
2016-08-17 9:35 ` Chris Wilson
@ 2016-08-18 9:56 ` Daniel Vetter
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2016-08-18 9:56 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Wed, Aug 17, 2016 at 10:35:31AM +0100, Chris Wilson wrote:
> On Wed, Aug 17, 2016 at 12:25:40PM +0300, David Weinehall wrote:
> > On Wed, Aug 17, 2016 at 09:08:58AM +0100, Chris Wilson wrote:
> > > On Wed, Aug 17, 2016 at 11:02:32AM +0300, David Weinehall wrote:
> > > > Since the workaround for buggy displays that do not reply to
> > > > live status detect immediately affects a rather limited set of
> > > > displays, and since the price paid (almost 100ms per HDMI-port),
> > > > we should have that hack disabled by default.
> > > >
> > > > Rather than leaving people with these kinds of broken displays
> > > > out in the cold completely, add a module parameter, defaulting
> > > > to -1 (live status detection on supported platforms, but without
> > > > the extra delays), that allows for re-enabling this hack.
> > >
> > > No. It is a regression. We revert back to the previous status quo,
> > > and then try to introduce live-status checking in a way that works if at
> > > all possible.
> >
> > The way I understand it, the only approaches that would allow for
> > combining live status checking with buggy displays are:
>
> I haven't seen any convincing analysis as to why live-status works
> better than ddc 0x50. Certainly not in the changelogs.
>
> The rule is that even if you fix one system, if you break anything else
> in the process and you cannot fix it promptly you revert back to the
> previous state and try again.
+1, and mine counts a thousands since magic maintainer powers.
We added this to make it hdmi complaint, because vpg asked for it. It
breaks shit, out it goes again. End of discussion.
The linux community is _very_ clear that when a standard disagrees with
experimental reality, reality wins. On top of that I'm very clear that
we're not going to add module parameters to fine-tune things to appease
different people and groups. Either it works, and we enable it, or it
doesn't, and then we should throw it out again. There's some grey area
with big features like fbc/psr where it makes sense to keep the code until
it's fixed, but definitely not for something as small as this here.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-08-18 9:56 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-17 8:02 [PATCH 0/2] Live status detection fixes David Weinehall
2016-08-17 8:02 ` [PATCH 1/2] drm/i915: Skip live status if not supported David Weinehall
2016-08-17 8:02 ` [PATCH 2/2] drm/i915: add module param for live_status David Weinehall
2016-08-17 8:08 ` Chris Wilson
2016-08-17 9:25 ` David Weinehall
2016-08-17 9:35 ` Chris Wilson
2016-08-18 9:56 ` Daniel Vetter
2016-08-17 8:35 ` ✗ Ro.CI.BAT: failure for Live status detection fixes Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.