| Message ID | 20161124094752.19129-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> == Series Details == > > Series: drm/i915/debugfs: Increment return value of gt.next_seqno > URL : https://patchwork.freedesktop.org/series/15887/ > State : warning > > == Summary == > > Series 15887v1 drm/i915/debugfs: Increment return value of > gt.next_seqno > https://patchwork.freedesktop.org/api/1.0/series/15887/revisions/1/mbo > x/ > > Test kms_force_connector_basic: > Subgroup force-connector-state: > pass -> DMESG-WARN (fi-snb-2520m) *ERROR* EDID checksum is invalid, remainder is 204 => https://bugs.freedesktop.org/show_bug.cgi?id=98625 > Test kms_pipe_crc_basic: > Subgroup nonblocking-crc-pipe-a: > pass -> DMESG-WARN (fi-ilk-650) https://bugs.freedesktop.org/show_bug.cgi?id=98251 but now on pipe-a. > > fi-bdw-5557u total:244 pass:229 dwarn:0 dfail:0 fail:0 skip:15 > fi-bsw-n3050 total:244 pass:204 dwarn:0 dfail:0 fail:0 skip:40 > fi-byt-j1900 total:244 pass:216 dwarn:0 dfail:0 fail:0 skip:28 > fi-byt-n2820 total:244 pass:212 dwarn:0 dfail:0 fail:0 skip:32 > fi-hsw-4770 total:244 pass:224 dwarn:0 dfail:0 fail:0 skip:20 > fi-hsw-4770r total:244 pass:224 dwarn:0 dfail:0 fail:0 skip:20 > fi-ilk-650 total:244 pass:190 dwarn:1 dfail:0 fail:0 skip:53 > fi-ivb-3520m total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22 > fi-ivb-3770 total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22 > fi-kbl-7200u total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22 > fi-skl-6260u total:244 pass:230 dwarn:0 dfail:0 fail:0 skip:14 > fi-skl-6700hq total:244 pass:223 dwarn:0 dfail:0 fail:0 skip:21 > fi-skl-6700k total:244 pass:222 dwarn:1 dfail:0 fail:0 skip:21 > fi-skl-6770hq total:244 pass:230 dwarn:0 dfail:0 fail:0 skip:14 > fi-snb-2520m total:244 pass:211 dwarn:1 dfail:0 fail:0 skip:32 > fi-snb-2600 total:244 pass:211 dwarn:0 dfail:0 fail:0 skip:33 > > 2154a854b8c9f732ef446a5c99cb24b8d97347f7 drm-intel-nightly: 2016y- > 11m-24d-07h-21m-33s UTC integration manifest > 43dccf2 drm/i915/debugfs: Increment return value of gt.next_seqno > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3104/ Jani Saarinen Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
On to, 2016-11-24 at 09:47 +0000, Chris Wilson wrote: > The i915_next_seqno read value is to be the next seqno used by the > kernel. However, in the conversion to atomics ops for gt.next_seqno, in > commit 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during > request allocation"), this was changed from a post-increment to a > pre-increment. This increment was missed from the value reported by > debugfs, so in effect it was reporting the current seqno (last > assigned), not the next seqno. > > Fixes: 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation") > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81209 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> <SNIP> > @@ -1026,7 +1026,7 @@ i915_next_seqno_get(void *data, u64 *val) > { > struct drm_i915_private *dev_priv = data; > > - *val = atomic_read(&dev_priv->gt.global_timeline.next_seqno); > + *val = 1 + atomic_read(&dev_priv->gt.global_timeline.next_seqno); The variable name really should be last_seqno or so... But as a hotfix; Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, joonas
On Thu, Nov 24, 2016 at 02:15:07PM +0200, Joonas Lahtinen wrote: > On to, 2016-11-24 at 09:47 +0000, Chris Wilson wrote: > > The i915_next_seqno read value is to be the next seqno used by the > > kernel. However, in the conversion to atomics ops for gt.next_seqno, in > > commit 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during > > request allocation"), this was changed from a post-increment to a > > pre-increment. This increment was missed from the value reported by > > debugfs, so in effect it was reporting the current seqno (last > > assigned), not the next seqno. > > > > Fixes: 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation") > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81209 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > <SNIP> > > > @@ -1026,7 +1026,7 @@ i915_next_seqno_get(void *data, u64 *val) > > { > > struct drm_i915_private *dev_priv = data; > > > > - *val = atomic_read(&dev_priv->gt.global_timeline.next_seqno); > > + *val = 1 + atomic_read(&dev_priv->gt.global_timeline.next_seqno); > > The variable name really should be last_seqno or so... I'd sign off on a s/timeline.next_seqno/timeline.seqno/. -Chris
On Thu, Nov 24, 2016 at 02:15:07PM +0200, Joonas Lahtinen wrote: > On to, 2016-11-24 at 09:47 +0000, Chris Wilson wrote: > > The i915_next_seqno read value is to be the next seqno used by the > > kernel. However, in the conversion to atomics ops for gt.next_seqno, in > > commit 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during > > request allocation"), this was changed from a post-increment to a > > pre-increment. This increment was missed from the value reported by > > debugfs, so in effect it was reporting the current seqno (last > > assigned), not the next seqno. > > > > Fixes: 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation") > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81209 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > <SNIP> > > > @@ -1026,7 +1026,7 @@ i915_next_seqno_get(void *data, u64 *val) > > { > > struct drm_i915_private *dev_priv = data; > > > > - *val = atomic_read(&dev_priv->gt.global_timeline.next_seqno); > > + *val = 1 + atomic_read(&dev_priv->gt.global_timeline.next_seqno); > > The variable name really should be last_seqno or so... > > But as a hotfix; > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Thanks, pushed and another (albeit recent) bug closed. -Chris
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 4cad5bf116b5..7b0fcd7461ea 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1026,7 +1026,7 @@ i915_next_seqno_get(void *data, u64 *val) { struct drm_i915_private *dev_priv = data; - *val = atomic_read(&dev_priv->gt.global_timeline.next_seqno); + *val = 1 + atomic_read(&dev_priv->gt.global_timeline.next_seqno); return 0; }
The i915_next_seqno read value is to be the next seqno used by the kernel. However, in the conversion to atomics ops for gt.next_seqno, in commit 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation"), this was changed from a post-increment to a pre-increment. This increment was missed from the value reported by debugfs, so in effect it was reporting the current seqno (last assigned), not the next seqno. Fixes: 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81209 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)