On Thu, Sep 28, 2017 at 06:00:03PM +0300, 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> > --- > 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. > + */ > +}; Excellent. Reviewed-by: Petri Latvala <petri.latvala@xxxxxxxxx> > 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); > > -- > 2.11.0 > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
