[i-g-t] i915/gem_ctx_isolation: Ignore the low bits of BB_OFFSET

Submitted by Chris Wilson on Jan. 7, 2019, 12:41 p.m.

Details

Message ID 20190107124140.20183-1-chris@chris-wilson.co.uk
State New
Headers show
Series "i915/gem_ctx_isolation: Ignore the low bits of BB_OFFSET" ( rev: 1 ) in IGT

Not browsing as part of any series.

Commit Message

Chris Wilson Jan. 7, 2019, 12:41 p.m.
On Skylake, BB_OFFSET seems to be unstable. Since this is an
offset into the batch at the time of CS execution, it should be actively
written to as we read from the register so allow it a qword of
discrepancy (since the CS should be reading in qwords). This still
allows us to detect dirt across the rest of the register field, should
that be required.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_ctx_isolation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 058cf3ec1..78a244382 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -96,7 +96,7 @@  static const struct named_register {
 	{ "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
 	{ "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2 },
 	{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
-	{ "BB_OFFSET", GEN8, RCS0, 0x2158 },
+	{ "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
 	{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
 	{ "CS_GPR", GEN8, RCS0, 0x2600, 32 },
 	{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },

Comments

On 07/01/19 04:41, Chris Wilson wrote:
> On Skylake, BB_OFFSET seems to be unstable. Since this is an
> offset into the batch at the time of CS execution, it should be actively
> written to as we read from the register so allow it a qword of
> discrepancy (since the CS should be reading in qwords). This still
> allows us to detect dirt across the rest of the register field, should
> that be required.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   tests/i915/gem_ctx_isolation.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 058cf3ec1..78a244382 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -96,7 +96,7 @@ static const struct named_register {
>   	{ "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
>   	{ "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2 },
>   	{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
> -	{ "BB_OFFSET", GEN8, RCS0, 0x2158 },
> +	{ "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },

The batch offset starts at bit 2. Do we observe changes in bit 0-1 as well?

Antonio

>   	{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
>   	{ "CS_GPR", GEN8, RCS0, 0x2600, 32 },
>   	{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
>
Quoting Antonio Argenziano (2019-01-10 21:24:56)
> 
> 
> On 07/01/19 04:41, Chris Wilson wrote:
> > On Skylake, BB_OFFSET seems to be unstable. Since this is an
> > offset into the batch at the time of CS execution, it should be actively
> > written to as we read from the register so allow it a qword of
> > discrepancy (since the CS should be reading in qwords). This still
> > allows us to detect dirt across the rest of the register field, should
> > that be required.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   tests/i915/gem_ctx_isolation.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> > index 058cf3ec1..78a244382 100644
> > --- a/tests/i915/gem_ctx_isolation.c
> > +++ b/tests/i915/gem_ctx_isolation.c
> > @@ -96,7 +96,7 @@ static const struct named_register {
> >       { "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
> >       { "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2 },
> >       { "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
> > -     { "BB_OFFSET", GEN8, RCS0, 0x2158 },
> > +     { "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
> 
> The batch offset starts at bit 2. Do we observe changes in bit 0-1 as well?

Not, it is just off by bit 2 (0x4). Bit 0 is also set when I don't
really expect it to be, I guess I really should just read what the
register is meant to be rather than guessing solely on the basis of its
name.
-Chris
Quoting Chris Wilson (2019-01-10 21:27:54)
> Quoting Antonio Argenziano (2019-01-10 21:24:56)
> > 
> > 
> > On 07/01/19 04:41, Chris Wilson wrote:
> > > On Skylake, BB_OFFSET seems to be unstable. Since this is an
> > > offset into the batch at the time of CS execution, it should be actively
> > > written to as we read from the register so allow it a qword of
> > > discrepancy (since the CS should be reading in qwords). This still
> > > allows us to detect dirt across the rest of the register field, should
> > > that be required.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >   tests/i915/gem_ctx_isolation.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> > > index 058cf3ec1..78a244382 100644
> > > --- a/tests/i915/gem_ctx_isolation.c
> > > +++ b/tests/i915/gem_ctx_isolation.c
> > > @@ -96,7 +96,7 @@ static const struct named_register {
> > >       { "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
> > >       { "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2 },
> > >       { "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
> > > -     { "BB_OFFSET", GEN8, RCS0, 0x2158 },
> > > +     { "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
> > 
> > The batch offset starts at bit 2. Do we observe changes in bit 0-1 as well?
> 
> Not, it is just off by bit 2 (0x4). Bit 0 is also set when I don't
> really expect it to be, I guess I really should just read what the
> register is meant to be rather than guessing solely on the basis of its
> name.

Bit 2 flip flops between reference value and observed (test failure).

Bit 0 simply differs from my own expectations.
-Chris
On 10/01/19 13:29, Chris Wilson wrote:
> Quoting Chris Wilson (2019-01-10 21:27:54)
>> Quoting Antonio Argenziano (2019-01-10 21:24:56)
>>>
>>>
>>> On 07/01/19 04:41, Chris Wilson wrote:
>>>> On Skylake, BB_OFFSET seems to be unstable. Since this is an
>>>> offset into the batch at the time of CS execution, it should be actively
>>>> written to as we read from the register so allow it a qword of
>>>> discrepancy (since the CS should be reading in qwords). This still
>>>> allows us to detect dirt across the rest of the register field, should
>>>> that be required.
>>>>
>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>> ---
>>>>    tests/i915/gem_ctx_isolation.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
>>>> index 058cf3ec1..78a244382 100644
>>>> --- a/tests/i915/gem_ctx_isolation.c
>>>> +++ b/tests/i915/gem_ctx_isolation.c
>>>> @@ -96,7 +96,7 @@ static const struct named_register {
>>>>        { "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
>>>>        { "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2 },
>>>>        { "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
>>>> -     { "BB_OFFSET", GEN8, RCS0, 0x2158 },
>>>> +     { "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
>>>
>>> The batch offset starts at bit 2. Do we observe changes in bit 0-1 as well?
>>
>> Not, it is just off by bit 2 (0x4). Bit 0 is also set when I don't
>> really expect it to be, I guess I really should just read what the
>> register is meant to be rather than guessing solely on the basis of its
>> name.
> 
> Bit 2 flip flops between reference value and observed (test failure).
> 
> Bit 0 simply differs from my own expectations.

I guess if it gets overwritten we catch it even if we ignore the lowest 
3 bits but something weird would have happened if 0-1 change.

With or without modifying the mask,
Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>

> -Chris
>
Quoting Antonio Argenziano (2019-01-10 21:58:52)
> 
> 
> On 10/01/19 13:29, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-01-10 21:27:54)
> >> Quoting Antonio Argenziano (2019-01-10 21:24:56)
> >>>
> >>>
> >>> On 07/01/19 04:41, Chris Wilson wrote:
> >>>> On Skylake, BB_OFFSET seems to be unstable. Since this is an
> >>>> offset into the batch at the time of CS execution, it should be actively
> >>>> written to as we read from the register so allow it a qword of
> >>>> discrepancy (since the CS should be reading in qwords). This still
> >>>> allows us to detect dirt across the rest of the register field, should
> >>>> that be required.
> >>>>
> >>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>> ---
> >>>>    tests/i915/gem_ctx_isolation.c | 2 +-
> >>>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> >>>> index 058cf3ec1..78a244382 100644
> >>>> --- a/tests/i915/gem_ctx_isolation.c
> >>>> +++ b/tests/i915/gem_ctx_isolation.c
> >>>> @@ -96,7 +96,7 @@ static const struct named_register {
> >>>>        { "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
> >>>>        { "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2 },
> >>>>        { "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
> >>>> -     { "BB_OFFSET", GEN8, RCS0, 0x2158 },
> >>>> +     { "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
> >>>
> >>> The batch offset starts at bit 2. Do we observe changes in bit 0-1 as well?
> >>
> >> Not, it is just off by bit 2 (0x4). Bit 0 is also set when I don't
> >> really expect it to be, I guess I really should just read what the
> >> register is meant to be rather than guessing solely on the basis of its
> >> name.
> > 
> > Bit 2 flip flops between reference value and observed (test failure).
> > 
> > Bit 0 simply differs from my own expectations.
> 
> I guess if it gets overwritten we catch it even if we ignore the lowest 
> 3 bits but something weird would have happened if 0-1 change.
> 
> With or without modifying the mask,
> Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>

Restricted the mask to .ignore_bits=0x4 so that we only ignore the bit
that is fluctuating in testing.

Thanks,
-Chris