Oscar Mateo <oscar.mateo@xxxxxxxxx> writes: > On 09/28/2017 08:00 AM, Mika Kuoppala wrote: >> We have no means to check write only registers as >> this would need access through context image. For now we >> know that cnl has a one such register, 0xe5f0 which is used >> to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting >> the context image without and with workaround applied: >> >> 0x0000a840: 0x0000e5f0 0xffff0800 >> 0x0000a840: 0x0000e5f0 0xffff0820 >> >> we can conclude that the workaround setup is working right >> in this particular case. >> >> Add a write only list and add register 0xe5f0 into this list. >> This is a temporary solution until a more capable context image >> checker emerges. >> >> v2: add code comment about adhocness (Petri) >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943 >> Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx> >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> >> Cc: Petri Latvala <petri.latvala@xxxxxxxxx> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> >> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> --- > > Acked-by: Oscar Mateo <oscar.mateo@xxxxxxxxx> Pushed, thanks for reviews and ack. -Mika > >> tests/gem_workarounds.c | 38 +++++++++++++++++++++++++++++++++++++- >> 1 file changed, 37 insertions(+), 1 deletion(-) >> >> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c >> index d6641bd5..5e30a7b8 100644 >> --- a/tests/gem_workarounds.c >> +++ b/tests/gem_workarounds.c >> @@ -29,6 +29,8 @@ >> >> #include <fcntl.h> >> >> +static int gen; >> + >> enum operation { >> GPU_RESET, >> SUSPEND_RESUME, >> @@ -41,6 +43,21 @@ struct intel_wa_reg { >> uint32_t mask; >> }; >> >> +static struct write_only_list { >> + unsigned int gen; >> + uint32_t addr; >> +} wo_list[] = { >> + { 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */ >> + >> + /* >> + * FIXME: If you are contemplating adding stuff here >> + * consider this as a temporary solution. You need to >> + * manually check from context image that your workaround >> + * is having an effect. Consider creating a context image >> + * validator to act as a superior solution. >> + */ >> +}; >> + >> static struct intel_wa_reg *wa_regs; >> static int num_wa_regs; >> >> @@ -64,6 +81,21 @@ static void test_suspend_resume(void) >> igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE); >> } >> >> +static bool write_only(const uint32_t addr) >> +{ >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(wo_list); i++) { >> + if (gen == wo_list[i].gen && >> + addr == wo_list[i].addr) { >> + igt_info("Skipping check for 0x%x due to write only\n", addr); >> + return true; >> + } >> + } >> + >> + return false; >> +} >> + >> static int workaround_fail_count(void) >> { >> int i, fail_count = 0; >> @@ -85,6 +117,9 @@ static int workaround_fail_count(void) >> wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask, >> val, ok ? "OK" : "FAIL"); >> >> + if (write_only(wa_regs[i].addr)) >> + continue; >> + >> if (!ok) { >> igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n", >> wa_regs[i].addr, wa_regs[i].value, >> @@ -124,7 +159,6 @@ igt_main >> { >> igt_fixture { >> int device = drm_open_driver_master(DRIVER_INTEL); >> - const int gen = intel_gen(intel_get_drm_devid(device)); >> struct pci_device *pci_dev; >> FILE *file; >> char *line = NULL; >> @@ -133,6 +167,8 @@ igt_main >> >> igt_require_gem(device); >> >> + gen = intel_gen(intel_get_drm_devid(device)); >> + >> pci_dev = intel_get_pci_device(); >> igt_require(pci_dev); >> _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
