Message ID | 1452261080-6979-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Fri, Jan 08, 2016 at 03:51:19PM +0200, Mika Kuoppala wrote: > With commit 8ac3e1bb76cc ("drm/i915: Add non claimed mmio checking > for vlv/chv") we now have chv/vlv support in place for detecting > unclaimed access. Also the perf hit of extra mmio read > is now only suffered if mmio_debug is set. > > This allows us to stuff the macro for unclaimed reg > detection inside a generic gen6 register access, as now all > gens using these macros uses also unclaimed debugs, the one > exception being snb. We gain more clean and generic macros > and only downside is that snb will suffer one branch perf hit > without upside. > > Note that the hsw write path debug register check now > happens before fifo check, but this should not make > any real difference. > > As vlv/chv use the generic gen6 access macros, the consequence > is that they gain the mmio_debug feature. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> I still fancy a GEN6_DBG_READ/WRITE_HEADER but with only a couple of gen, it doesn't seem worth the argument. (Though we still need to reduce the expense of ilk_irq_handler, but for the most part that is to avoid these vfunc altogether.) Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
Patchwork <patchwork@annarchy.freedesktop.org> writes: > == Summary == > > Built on ff88655b3a5467bbc3be8c67d3e05ebf182557d3 drm-intel-nightly: 2016y-01m-11d-07h-30m-16s UTC integration manifest > > Test gem_storedw_loop: > Subgroup basic-render: > pass -> DMESG-WARN (skl-i5k-2) UNSTABLE > dmesg-warn -> PASS (bdw-ultra) > Test kms_flip: > Subgroup basic-flip-vs-dpms: > dmesg-warn -> PASS (ilk-hp8440p) > Subgroup basic-flip-vs-modeset: > pass -> DMESG-WARN (skl-i5k-2) > dmesg-warn -> PASS (bsw-nuc-2) > pass -> DMESG-WARN (ilk-hp8440p) > dmesg-warn -> PASS (byt-nuc) > Subgroup basic-flip-vs-wf_vblank: > pass -> DMESG-WARN (byt-nuc) > Subgroup basic-plain-flip: > dmesg-warn -> PASS (bsw-nuc-2) > dmesg-warn -> PASS (byt-nuc) > Test kms_pipe_crc_basic: > Subgroup nonblocking-crc-pipe-b: > pass -> DMESG-WARN (byt-nuc) > Subgroup nonblocking-crc-pipe-c: > pass -> DMESG-WARN (bsw-nuc-2) > Subgroup read-crc-pipe-a: > dmesg-warn -> PASS (byt-nuc) > Subgroup read-crc-pipe-b: > dmesg-warn -> PASS (byt-nuc) > Subgroup read-crc-pipe-b-frame-sequence: > pass -> DMESG-WARN (bdw-ultra) > dmesg-warn -> PASS (byt-nuc) > Subgroup read-crc-pipe-c: > dmesg-warn -> PASS (bsw-nuc-2) > Subgroup read-crc-pipe-c-frame-sequence: > pass -> DMESG-WARN (bsw-nuc-2) > Test pm_rpm: > Subgroup basic-pci-d3-state: > dmesg-warn -> PASS (byt-nuc) > > bdw-ultra total:138 pass:130 dwarn:1 dfail:0 fail:1 > skip:6 Tomi suspected this as a fluke fail. There was no igt stdout nor stderr for this case. We did a rerun and bdw-ultra passed. -Mika > bsw-nuc-2 total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 > byt-nuc total:141 pass:122 dwarn:4 dfail:0 fail:0 skip:15 > hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 > hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 > ilk-hp8440p total:141 pass:100 dwarn:4 dfail:0 fail:0 skip:37 > skl-i5k-2 total:141 pass:130 dwarn:3 dfail:0 fail:0 skip:8 > skl-i7k-2 total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 > snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 > snb-x220t total:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 > > HANGED ivb-t430s in igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b > > Results at /archive/results/CI_IGT_test/Patchwork_1116/
Chris Wilson <chris@chris-wilson.co.uk> writes: > On Fri, Jan 08, 2016 at 03:51:19PM +0200, Mika Kuoppala wrote: >> With commit 8ac3e1bb76cc ("drm/i915: Add non claimed mmio checking >> for vlv/chv") we now have chv/vlv support in place for detecting >> unclaimed access. Also the perf hit of extra mmio read >> is now only suffered if mmio_debug is set. >> >> This allows us to stuff the macro for unclaimed reg >> detection inside a generic gen6 register access, as now all >> gens using these macros uses also unclaimed debugs, the one >> exception being snb. We gain more clean and generic macros >> and only downside is that snb will suffer one branch perf hit >> without upside. >> >> Note that the hsw write path debug register check now >> happens before fifo check, but this should not make >> any real difference. >> >> As vlv/chv use the generic gen6 access macros, the consequence >> is that they gain the mmio_debug feature. >> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > > I still fancy a GEN6_DBG_READ/WRITE_HEADER but with only a couple of > gen, it doesn't seem worth the argument. (Though we still need to reduce > the expense of ilk_irq_handler, but for the most part that is to avoid > these vfunc altogether.) > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > -Chris Both patches pushed to dinq. Thankyou for review. -Mika
On Mon, Jan 11, 2016 at 02:50:28PM +0200, Mika Kuoppala wrote: > Patchwork <patchwork@annarchy.freedesktop.org> writes: > > > == Summary == > > > > Built on ff88655b3a5467bbc3be8c67d3e05ebf182557d3 drm-intel-nightly: 2016y-01m-11d-07h-30m-16s UTC integration manifest > > > > Test gem_storedw_loop: > > Subgroup basic-render: > > pass -> DMESG-WARN (skl-i5k-2) UNSTABLE > > dmesg-warn -> PASS (bdw-ultra) > > Test kms_flip: > > Subgroup basic-flip-vs-dpms: > > dmesg-warn -> PASS (ilk-hp8440p) > > Subgroup basic-flip-vs-modeset: > > pass -> DMESG-WARN (skl-i5k-2) > > dmesg-warn -> PASS (bsw-nuc-2) > > pass -> DMESG-WARN (ilk-hp8440p) > > dmesg-warn -> PASS (byt-nuc) > > Subgroup basic-flip-vs-wf_vblank: > > pass -> DMESG-WARN (byt-nuc) > > Subgroup basic-plain-flip: > > dmesg-warn -> PASS (bsw-nuc-2) > > dmesg-warn -> PASS (byt-nuc) > > Test kms_pipe_crc_basic: > > Subgroup nonblocking-crc-pipe-b: > > pass -> DMESG-WARN (byt-nuc) > > Subgroup nonblocking-crc-pipe-c: > > pass -> DMESG-WARN (bsw-nuc-2) > > Subgroup read-crc-pipe-a: > > dmesg-warn -> PASS (byt-nuc) > > Subgroup read-crc-pipe-b: > > dmesg-warn -> PASS (byt-nuc) > > Subgroup read-crc-pipe-b-frame-sequence: > > pass -> DMESG-WARN (bdw-ultra) > > dmesg-warn -> PASS (byt-nuc) > > Subgroup read-crc-pipe-c: > > dmesg-warn -> PASS (bsw-nuc-2) > > Subgroup read-crc-pipe-c-frame-sequence: > > pass -> DMESG-WARN (bsw-nuc-2) But what with all the pass->dmesg-warn changes above? That's considered BAT failure too, we can't afford to sprinkle warnings all over ... And it's a bunch of different machines. > > Test pm_rpm: > > Subgroup basic-pci-d3-state: > > dmesg-warn -> PASS (byt-nuc) > > > > bdw-ultra total:138 pass:130 dwarn:1 dfail:0 fail:1 > > skip:6 > > Tomi suspected this as a fluke fail. There was no igt stdout nor stderr > for this case. We did a rerun and bdw-ultra passed. > > -Mika > > > bsw-nuc-2 total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 > > byt-nuc total:141 pass:122 dwarn:4 dfail:0 fail:0 skip:15 > > hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 > > hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 > > ilk-hp8440p total:141 pass:100 dwarn:4 dfail:0 fail:0 skip:37 > > skl-i5k-2 total:141 pass:130 dwarn:3 dfail:0 fail:0 skip:8 > > skl-i7k-2 total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 > > snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 > > snb-x220t total:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 > > > > HANGED ivb-t430s in igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b So is killing a machine ... pretty sure this is new-ish. -Daniel > > > > Results at /archive/results/CI_IGT_test/Patchwork_1116/ > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter <daniel@ffwll.ch> writes: > On Mon, Jan 11, 2016 at 02:50:28PM +0200, Mika Kuoppala wrote: >> Patchwork <patchwork@annarchy.freedesktop.org> writes: >> >> > == Summary == >> > >> > Built on ff88655b3a5467bbc3be8c67d3e05ebf182557d3 drm-intel-nightly: 2016y-01m-11d-07h-30m-16s UTC integration manifest >> > >> > Test gem_storedw_loop: >> > Subgroup basic-render: >> > pass -> DMESG-WARN (skl-i5k-2) UNSTABLE >> > dmesg-warn -> PASS (bdw-ultra) >> > Test kms_flip: >> > Subgroup basic-flip-vs-dpms: >> > dmesg-warn -> PASS (ilk-hp8440p) >> > Subgroup basic-flip-vs-modeset: >> > pass -> DMESG-WARN (skl-i5k-2) >> > dmesg-warn -> PASS (bsw-nuc-2) >> > pass -> DMESG-WARN (ilk-hp8440p) >> > dmesg-warn -> PASS (byt-nuc) >> > Subgroup basic-flip-vs-wf_vblank: >> > pass -> DMESG-WARN (byt-nuc) >> > Subgroup basic-plain-flip: >> > dmesg-warn -> PASS (bsw-nuc-2) >> > dmesg-warn -> PASS (byt-nuc) >> > Test kms_pipe_crc_basic: >> > Subgroup nonblocking-crc-pipe-b: >> > pass -> DMESG-WARN (byt-nuc) >> > Subgroup nonblocking-crc-pipe-c: >> > pass -> DMESG-WARN (bsw-nuc-2) >> > Subgroup read-crc-pipe-a: >> > dmesg-warn -> PASS (byt-nuc) >> > Subgroup read-crc-pipe-b: >> > dmesg-warn -> PASS (byt-nuc) >> > Subgroup read-crc-pipe-b-frame-sequence: >> > pass -> DMESG-WARN (bdw-ultra) >> > dmesg-warn -> PASS (byt-nuc) >> > Subgroup read-crc-pipe-c: >> > dmesg-warn -> PASS (bsw-nuc-2) >> > Subgroup read-crc-pipe-c-frame-sequence: >> > pass -> DMESG-WARN (bsw-nuc-2) > > But what with all the pass->dmesg-warn changes above? That's considered > BAT failure too, we can't afford to sprinkle warnings all over ... And > it's a bunch of different machines. > >> > Test pm_rpm: >> > Subgroup basic-pci-d3-state: >> > dmesg-warn -> PASS (byt-nuc) >> > >> > bdw-ultra total:138 pass:130 dwarn:1 dfail:0 fail:1 >> > skip:6 >> >> Tomi suspected this as a fluke fail. There was no igt stdout nor stderr >> for this case. We did a rerun and bdw-ultra passed. >> >> -Mika >> >> > bsw-nuc-2 total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 >> > byt-nuc total:141 pass:122 dwarn:4 dfail:0 fail:0 skip:15 >> > hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 >> > hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 >> > ilk-hp8440p total:141 pass:100 dwarn:4 dfail:0 fail:0 skip:37 >> > skl-i5k-2 total:141 pass:130 dwarn:3 dfail:0 fail:0 skip:8 >> > skl-i7k-2 total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 >> > snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 >> > snb-x220t total:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 >> > >> > HANGED ivb-t430s in igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b > > So is killing a machine ... pretty sure this is new-ish. > -Daniel > I did cover my back with this one. This happened on other sets on the same day and I went and asked Tomi. He said that don't worry about it. So I didn't. -Mika > >> > >> > Results at /archive/results/CI_IGT_test/Patchwork_1116/ >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Daniel Vetter <daniel@ffwll.ch> writes: > On Mon, Jan 11, 2016 at 02:50:28PM +0200, Mika Kuoppala wrote: >> Patchwork <patchwork@annarchy.freedesktop.org> writes: >> >> > == Summary == >> > >> > Built on ff88655b3a5467bbc3be8c67d3e05ebf182557d3 drm-intel-nightly: 2016y-01m-11d-07h-30m-16s UTC integration manifest >> > >> > Test gem_storedw_loop: >> > Subgroup basic-render: >> > pass -> DMESG-WARN (skl-i5k-2) UNSTABLE >> > dmesg-warn -> PASS (bdw-ultra) >> > Test kms_flip: >> > Subgroup basic-flip-vs-dpms: >> > dmesg-warn -> PASS (ilk-hp8440p) >> > Subgroup basic-flip-vs-modeset: >> > pass -> DMESG-WARN (skl-i5k-2) >> > dmesg-warn -> PASS (bsw-nuc-2) >> > pass -> DMESG-WARN (ilk-hp8440p) >> > dmesg-warn -> PASS (byt-nuc) >> > Subgroup basic-flip-vs-wf_vblank: >> > pass -> DMESG-WARN (byt-nuc) >> > Subgroup basic-plain-flip: >> > dmesg-warn -> PASS (bsw-nuc-2) >> > dmesg-warn -> PASS (byt-nuc) >> > Test kms_pipe_crc_basic: >> > Subgroup nonblocking-crc-pipe-b: >> > pass -> DMESG-WARN (byt-nuc) >> > Subgroup nonblocking-crc-pipe-c: >> > pass -> DMESG-WARN (bsw-nuc-2) >> > Subgroup read-crc-pipe-a: >> > dmesg-warn -> PASS (byt-nuc) >> > Subgroup read-crc-pipe-b: >> > dmesg-warn -> PASS (byt-nuc) >> > Subgroup read-crc-pipe-b-frame-sequence: >> > pass -> DMESG-WARN (bdw-ultra) >> > dmesg-warn -> PASS (byt-nuc) >> > Subgroup read-crc-pipe-c: >> > dmesg-warn -> PASS (bsw-nuc-2) >> > Subgroup read-crc-pipe-c-frame-sequence: >> > pass -> DMESG-WARN (bsw-nuc-2) > > But what with all the pass->dmesg-warn changes above? That's considered > BAT failure too, we can't afford to sprinkle warnings all over ... And > it's a bunch of different machines. > Forgot to address this one in previous mail. This patchset added more debug infra and enabled it for bsw/byt. So assumpion is that it did uncover a real problem thus the warns. Is the policy that if your debug infra reveals problems, you need to fix those problems? If so, there is a chicken and egg problem as you don't always have access to hardware that your debug infra will cover. Thanks, -Mika >> > Test pm_rpm: >> > Subgroup basic-pci-d3-state: >> > dmesg-warn -> PASS (byt-nuc) >> > >> > bdw-ultra total:138 pass:130 dwarn:1 dfail:0 fail:1 >> > skip:6 >> >> Tomi suspected this as a fluke fail. There was no igt stdout nor stderr >> for this case. We did a rerun and bdw-ultra passed. >> >> -Mika >> >> > bsw-nuc-2 total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 >> > byt-nuc total:141 pass:122 dwarn:4 dfail:0 fail:0 skip:15 >> > hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 >> > hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 >> > ilk-hp8440p total:141 pass:100 dwarn:4 dfail:0 fail:0 skip:37 >> > skl-i5k-2 total:141 pass:130 dwarn:3 dfail:0 fail:0 skip:8 >> > skl-i7k-2 total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 >> > snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 >> > snb-x220t total:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 >> > >> > HANGED ivb-t430s in igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b > > So is killing a machine ... pretty sure this is new-ish. > -Daniel > > >> > >> > Results at /archive/results/CI_IGT_test/Patchwork_1116/ >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Wed, Jan 13, 2016 at 10:20 AM, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote: >> But what with all the pass->dmesg-warn changes above? That's considered >> BAT failure too, we can't afford to sprinkle warnings all over ... And >> it's a bunch of different machines. >> > > Forgot to address this one in previous mail. This patchset > added more debug infra and enabled it for bsw/byt. So assumpion > is that it did uncover a real problem thus the warns. > > Is the policy that if your debug infra reveals problems, > you need to fix those problems? We should discuss this in the next meeting (adding Annie/Jesse for that), but personally I think adding new WARN/ERROR noise should never result in BAT regressions. If your patch uncovers existing failures even in BAT then imo the right approach is to first fix up things, then apply the WARN patch to make sure we don't break things. The problem is that once you have dmesg noise in a test, then no one will notice additional noise and regressions. And that's how we got into our mess last summer. Also dmesg noise is not really acceptable anyway and needs to be fixed (Linus/Dave will get grumpy). If that takes too long because there's lots of warn, then maybe we need to first add the new sanity check at debug level, just to help with tracking down issues. We might need to improve CI reporting so that debug level dmesg is still capture completely for BAT runs. > If so, there is a chicken and egg problem as you don't > always have access to hardware that your debug infra > will cover. Yeah, we need to enable manual submission to CI-machines. Abusing CI as a test facility simply means that you're ok with blocking everyone else with your CI result spam. -Daniel
Argh, actually add Annie/Jesse! -Daniel On Wed, Jan 13, 2016 at 10:39 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Wed, Jan 13, 2016 at 10:20 AM, Mika Kuoppala > <mika.kuoppala@linux.intel.com> wrote: >>> But what with all the pass->dmesg-warn changes above? That's considered >>> BAT failure too, we can't afford to sprinkle warnings all over ... And >>> it's a bunch of different machines. >>> >> >> Forgot to address this one in previous mail. This patchset >> added more debug infra and enabled it for bsw/byt. So assumpion >> is that it did uncover a real problem thus the warns. >> >> Is the policy that if your debug infra reveals problems, >> you need to fix those problems? > > We should discuss this in the next meeting (adding Annie/Jesse for > that), but personally I think adding new WARN/ERROR noise should never > result in BAT regressions. If your patch uncovers existing failures > even in BAT then imo the right approach is to first fix up things, > then apply the WARN patch to make sure we don't break things. The > problem is that once you have dmesg noise in a test, then no one will > notice additional noise and regressions. And that's how we got into > our mess last summer. > > Also dmesg noise is not really acceptable anyway and needs to be fixed > (Linus/Dave will get grumpy). > > If that takes too long because there's lots of warn, then maybe we > need to first add the new sanity check at debug level, just to help > with tracking down issues. We might need to improve CI reporting so > that debug level dmesg is still capture completely for BAT runs. > >> If so, there is a chicken and egg problem as you don't >> always have access to hardware that your debug infra >> will cover. > > Yeah, we need to enable manual submission to CI-machines. Abusing CI > as a test facility simply means that you're ok with blocking everyone > else with your CI result spam. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index d0973e08e7eb..0b47bc891dc7 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -626,14 +626,11 @@ ilk_dummy_write(struct drm_i915_private *dev_priv) } static void -hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, - const i915_reg_t reg, - const bool read, - const bool before) +__unclaimed_reg_debug(struct drm_i915_private *dev_priv, + const i915_reg_t reg, + const bool read, + const bool before) { - if (likely(!i915.mmio_debug)) - return; - if (WARN(check_for_unclaimed_mmio(dev_priv), "Unclaimed register detected %s %s register 0x%x\n", before ? "before" : "after", @@ -642,6 +639,18 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, i915.mmio_debug--; /* Only report the first N failures */ } +static inline void +unclaimed_reg_debug(struct drm_i915_private *dev_priv, + const i915_reg_t reg, + const bool read, + const bool before) +{ + if (likely(!i915.mmio_debug)) + return; + + __unclaimed_reg_debug(dev_priv, reg, read, before); +} + #define GEN2_READ_HEADER(x) \ u##x val = 0; \ assert_rpm_wakelock_held(dev_priv); @@ -687,9 +696,11 @@ __gen2_read(64) unsigned long irqflags; \ u##x val = 0; \ assert_rpm_wakelock_held(dev_priv); \ - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags) + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \ + unclaimed_reg_debug(dev_priv, reg, true, true) #define GEN6_READ_FOOTER \ + unclaimed_reg_debug(dev_priv, reg, true, false); \ spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \ trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \ return val @@ -722,11 +733,9 @@ static inline void __force_wake_get(struct drm_i915_private *dev_priv, static u##x \ gen6_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \ GEN6_READ_HEADER(x); \ - hsw_unclaimed_reg_debug(dev_priv, reg, true, true); \ if (NEEDS_FORCE_WAKE(offset)) \ __force_wake_get(dev_priv, FORCEWAKE_RENDER); \ val = __raw_i915_read##x(dev_priv, reg); \ - hsw_unclaimed_reg_debug(dev_priv, reg, true, false); \ GEN6_READ_FOOTER; \ } @@ -774,7 +783,6 @@ static u##x \ gen9_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \ enum forcewake_domains fw_engine; \ GEN6_READ_HEADER(x); \ - hsw_unclaimed_reg_debug(dev_priv, reg, true, true); \ if (!SKL_NEEDS_FORCE_WAKE(offset)) \ fw_engine = 0; \ else if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(offset)) \ @@ -788,7 +796,6 @@ gen9_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \ if (fw_engine) \ __force_wake_get(dev_priv, fw_engine); \ val = __raw_i915_read##x(dev_priv, reg); \ - hsw_unclaimed_reg_debug(dev_priv, reg, true, false); \ GEN6_READ_FOOTER; \ } @@ -887,9 +894,11 @@ __gen2_write(64) unsigned long irqflags; \ trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \ assert_rpm_wakelock_held(dev_priv); \ - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags) + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \ + unclaimed_reg_debug(dev_priv, reg, false, true) #define GEN6_WRITE_FOOTER \ + unclaimed_reg_debug(dev_priv, reg, false, false); \ spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags) #define __gen6_write(x) \ @@ -915,12 +924,10 @@ hsw_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool t if (NEEDS_FORCE_WAKE(offset)) { \ __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \ } \ - hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \ __raw_i915_write##x(dev_priv, reg, val); \ if (unlikely(__fifo_ret)) { \ gen6_gt_check_fifodbg(dev_priv); \ } \ - hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \ GEN6_WRITE_FOOTER; \ } @@ -950,11 +957,9 @@ static bool is_gen8_shadowed(struct drm_i915_private *dev_priv, static void \ gen8_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \ GEN6_WRITE_HEADER; \ - hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \ if (NEEDS_FORCE_WAKE(offset) && !is_gen8_shadowed(dev_priv, reg)) \ __force_wake_get(dev_priv, FORCEWAKE_RENDER); \ __raw_i915_write##x(dev_priv, reg, val); \ - hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \ GEN6_WRITE_FOOTER; \ } @@ -1008,7 +1013,6 @@ gen9_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, \ bool trace) { \ enum forcewake_domains fw_engine; \ GEN6_WRITE_HEADER; \ - hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \ if (!SKL_NEEDS_FORCE_WAKE(offset) || \ is_gen9_shadowed(dev_priv, reg)) \ fw_engine = 0; \ @@ -1023,7 +1027,6 @@ gen9_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, \ if (fw_engine) \ __force_wake_get(dev_priv, fw_engine); \ __raw_i915_write##x(dev_priv, reg, val); \ - hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \ GEN6_WRITE_FOOTER; \ }
With commit 8ac3e1bb76cc ("drm/i915: Add non claimed mmio checking for vlv/chv") we now have chv/vlv support in place for detecting unclaimed access. Also the perf hit of extra mmio read is now only suffered if mmio_debug is set. This allows us to stuff the macro for unclaimed reg detection inside a generic gen6 register access, as now all gens using these macros uses also unclaimed debugs, the one exception being snb. We gain more clean and generic macros and only downside is that snb will suffer one branch perf hit without upside. Note that the hsw write path debug register check now happens before fifo check, but this should not make any real difference. As vlv/chv use the generic gen6 access macros, the consequence is that they gain the mmio_debug feature. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/intel_uncore.c | 41 ++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 19 deletions(-)