[Intel-gfx] [RFC 2/2] igt/gem_workarounds: igt to test workaround registers

[Daniel Vetter]

On Fri, Aug 8, 2014 at 6:39 PM, Siluvery, Arun
<arun.siluvery at linux.intel.com> wrote:
>>
>> The suspend test doesn't seem to be wire up ...
>>
>> Also I think it would be worth to have a module-reload version here too.
>>
> Suspend/Resume is not working; device is not resuming even after the timer
> is elapsed. Do we know suspend/resume works correctly on nightly?

Should still work. You can run one of the basic tests in drv_suspend
to check, or run the underlying cmd used by the igt helper library.
Could very well be that the bios on your machine doesn't support
autoresume for some reasons, so manually waking up is useful to test.

>>> +};
>>> +
>>> +struct workaround {
>>> +       const char *reg_name;
>>> +       uint32_t address;
>>> +};
>>> +
>>> +static struct workaround bdw_workarounds[] =
>>> +{
>>> +       { "GEN8_ROW_CHICKEN", GEN8_ROW_CHICKEN },
>>> +       { "GEN7_ROW_CHICKEN2", GEN7_ROW_CHICKEN2 },
>>> +       { "HALF_SLICE_CHICKEN3", HALF_SLICE_CHICKEN3 },
>>> +       { "GEN7_HALF_SLICE_CHICKEN1", GEN7_HALF_SLICE_CHICKEN1 },
>>> +       { "COMMON_SLICE_CHICKEN2", COMMON_SLICE_CHICKEN2 },
>>> +       { "HDC_CHICKEN0", HDC_CHICKEN0 },
>>> +       { "GEN7_CACHE_MODE_1", GEN7_CACHE_MODE_1 },
>>> +       { "GEN7_GT_MODE", GEN7_GT_MODE },
>>> +       { "GAMTARBMODE", GAMTARBMODE },
>>> +       { "_3D_CHICKEN3", _3D_CHICKEN3 },
>>> +       { "GAM_ECOCHK", GAM_ECOCHK },
>>> +       { "CHICKEN_PAR1_1", CHICKEN_PAR1_1 },
>>> +       { "GEN7_FF_THREAD_MODE", GEN7_FF_THREAD_MODE },
>>> +       { "GEN6_RC_SLEEP_PSMI_CONTROL", GEN6_RC_SLEEP_PSMI_CONTROL },
>>> +       { "GEN8_UCGCTL6", GEN8_UCGCTL6 },
>>> +       { "NULL", 0xFFFF },
>>> +};
>>
>>
>> Crazy idea I've just had to validate that all the w/a table here is
>> up-to-date with the one in the kernel:
>>
>> - We create a special WA_REG macro in the kernel which we use to wrap all
>>    registers used in workarounds at the specific use-site (i.e. not in the
>>    header). So
>>
>>    I951_WRITE(WA_REG(GEN8_ROW_CHICKEN), ....);
>>
>> - That macro then adds the register to a table which we can dump through
>>    debugs with a file called intel_wa_registers. This happens at runtime.
>>    This is important since a static list over all platforms might included
>>    registers which hang some platforms when we read them.
>>
>> - A special subtest in this test here compares the kernel-provided list
>>    with the one supplied here and makes sure that all the w/a in the
>> kernel
>>    list are in the test list, too. Or we just ditch the test list here
>>    completely, but that might not work for special cases where we only
>> need
>>    to check some masks ...
>>
>> Opinions on this? Would this help with maintaining this testcase and
>> ensuring that it is always up-to-date with the kernel w/a list? I really
>> want to make sure we get this right, there's been way too many cases where
>> w/a settings have been lost over resume, runtime pm, ctx switches ...
>>
> I will change the implementation to use this macro.
> so in this case the table is updated before each use case (reset,
> suspend/resume, module reload etc)? Is it not sufficient to capture the
> state at the beginning? my understanding is the wa state should really stay
> the same and we compare the current state (eg after reset) to the one at the
> beginning rather than the state before reset.

Yeah, just grabbing the wa register list once from debugfs in an
igt_fixture block is good enough.

> I think it is easier to maintain if we completely remove the workaround list
> from igt itself, based on hardware macro can populate only those workarounds
> that are applicable but you mentioned that may not work for special cases,
> could you elaborate about these cases?

Some w/a are set in registers which have other stuff that changes at
runtime, iirc some of the ring-related registers are set in RING_CTL.
So for those we'd need a mask to make sure we exclude the bits that
vary. But we can solve that problem once we have it, your current list
doesn't have any such special case.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch