| Message ID | 20190107124140.20183-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t] i915/gem_ctx_isolation: Ignore the low bits of BB_OFFSET | expand |
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
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 },
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(-)