Re: [PATCH v3] test/gem_mocs_settings: Testing MOCS register settings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 11 Apr 2016, Chris Wilson wrote:

On Mon, Apr 11, 2016 at 04:02:17PM +0100, Peter Antoine wrote:
+	for (index = 0, offset = 0; index < size; index++, offset += 4)
+	{
+		batch[offset]   = MI_STORE_REGISTER_MEM_64_BIT_ADDR;
+		batch[offset+1] = reg_base + (index * sizeof(uint32_t));
+		batch[offset+2] = index * sizeof(uint32_t);	/* reloc */
+		batch[offset+3] = 0;
+
+		reloc[index].offset = (offset + 2) * sizeof(uint32_t);
+		reloc[index].delta = index * sizeof(uint32_t);
+		reloc[index].target_handle = dst_handle;
+		reloc[index].write_domain = I915_GEM_DOMAIN_RENDER;

Will cause an -EINVAL as you are specifing a write_domain but not the
corresponding read_domain.
Removed.

I hope you meant added the missing read_domains!

(Did not cause an error when tested)

Hmm, ok we don't actually warn about that!

BXT no error, SKL error (different versions of the kernel).
Added missing read_domains.

+static void do_read_registers(int fd,
+			      uint32_t ctx_id,
+			      uint32_t dst_handle,
+			      uint32_t reg_base,
+			      uint32_t size,
+			      uint32_t engine_id)
+{
+	gem_execbuf(fd, &execbuf);
+	gem_wait(fd, dst_handle, &timeout_ns);

As mentioned earlier, tried to avoid manual wait/sync so that we stress
the kernel harder to do the right thing. Sometimes the sync is required
as part of the test, but only rarely and should be well documented.
replaced with set_domain.


+	gem_close(fd, handle);
+}

+static void check_control_registers(int fd,
+				    const struct intel_execution_engine *engine,
+				    uint32_t ctx_id,
+				    bool dirty)
+{
+	uint32_t dst_handle = gem_create(fd, 4096);
+	uint32_t reg_base  = get_engine_base(engine->exec_id | engine->flags);
+	uint32_t *read_regs;
+	struct mocs_table table;
+
+	igt_assert(get_mocs_settings(intel_get_drm_devid(fd), &table, dirty));
+
+	do_read_registers(fd,
+			  ctx_id,
+			  dst_handle,
+			  reg_base,
+			  table.size,
+			  engine->exec_id | engine->flags);
+
+	read_regs = gem_mmap__cpu(fd, dst_handle, 0, 4096, PROT_READ);

Here you need the corresponding
gem_set_comdin(fd, dst_handle, DOMAIN_CPU, 0);
and in check_l3cc_registers.
Added into do_read_registers() so should handle both.

But less explicit and doesn't document the code as well. Move the
set-domain next to the usage of dst_handle in the CPU domain.
-Chris


Moved out of do_ream_domain()

Found an issue on SKL, it does not have the snoop bit that the BXT has, changed test so that the dirty check takes account of the different settable bits.

Version 4, coming now.

Peter.

--
   Peter Antoine (Android Graphics Driver Software Engineer)
   ---------------------------------------------------------------------
   Intel Corporation (UK) Limited
   Registered No. 1134945 (England)
   Registered Office: Pipers Way, Swindon SN3 1RJ
   VAT No: 860 2173 47
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux