[1/2] drm/i915: Enable mmio_debug for vlv/chv

Commit Message

Mika Kuoppala Jan. 8, 2016, 1:51 p.m. UTC

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ä <[e-mail]>
Cc: Chris Wilson <[e-mail]>
Signed-off-by: Mika Kuoppala <[e-mail]>
---
 drivers/gpu/drm/i915/intel_uncore.c | 41 ++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 19 deletions(-)

Comments

Chris Wilson Jan. 8, 2016, 2:20 p.m. UTC | #1

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ä <[e-mail]>
> Cc: Chris Wilson <[e-mail]>
> Signed-off-by: Mika Kuoppala <[e-mail]>

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 <[e-mail]>
-Chris

Mika Kuoppala Jan. 11, 2016, 12:50 p.m. UTC | #2

Patchwork <[e-mail]> 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/

Mika Kuoppala Jan. 11, 2016, 4:53 p.m. UTC | #3

Chris Wilson <[e-mail]> 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ä <[e-mail]>
>> Cc: Chris Wilson <[e-mail]>
>> Signed-off-by: Mika Kuoppala <[e-mail]>
>
> 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 <[e-mail]>
> -Chris

Both patches pushed to dinq. Thankyou for review.

-Mika

Daniel Vetter Jan. 12, 2016, 4:22 p.m. UTC | #4

On Mon, Jan 11, 2016 at 02:50:28PM +0200, Mika Kuoppala wrote:
> Patchwork <[e-mail]> 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
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Mika Kuoppala Jan. 13, 2016, 9:16 a.m. UTC | #5

Daniel Vetter <[e-mail]> writes:

> On Mon, Jan 11, 2016 at 02:50:28PM +0200, Mika Kuoppala wrote:
>> Patchwork <[e-mail]> 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
>> [e-mail]
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Mika Kuoppala Jan. 13, 2016, 9:20 a.m. UTC | #6

Daniel Vetter <[e-mail]> writes:

> On Mon, Jan 11, 2016 at 02:50:28PM +0200, Mika Kuoppala wrote:
>> Patchwork <[e-mail]> 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
>> [e-mail]
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Daniel Vetter Jan. 13, 2016, 9:39 a.m. UTC | #7

On Wed, Jan 13, 2016 at 10:20 AM, Mika Kuoppala
<[e-mail]> 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 Jan. 13, 2016, 9:39 a.m. UTC | #8

Argh, actually add Annie/Jesse!
-Daniel

On Wed, Jan 13, 2016 at 10:39 AM, Daniel Vetter <[e-mail]> wrote:
> On Wed, Jan 13, 2016 at 10:20 AM, Mika Kuoppala
> <[e-mail]> 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
> [phone] - http://blog.ffwll.ch

Patch

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; \
 }