Oscar Mateo <oscar.ma...@intel.com> 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.ma...@intel.com>
>> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
>> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
>> Cc: Petri Latvala <petri.latv...@intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuopp...@linux.intel.com>
>> Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk>
>> ---
>
> Acked-by: Oscar Mateo <oscar.ma...@intel.com>
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
