Re: [PATCH v6] igt/gem_trtt: Exercise the TRTT hardware

From: Chris Wilson <chris@chris-wilson.co.uk>
To: "Goel, Akash" <akash.goel@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v6] igt/gem_trtt: Exercise the TRTT hardware
Date: Fri, 18 Mar 2016 09:22:28 +0000	[thread overview]
Message-ID: <20160318092228.GR14143@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <56EBC3E3.5010900@intel.com>

On Fri, Mar 18, 2016 at 02:31:23PM +0530, Goel, Akash wrote:
> 
> 
> On 3/18/2016 2:06 PM, Chris Wilson wrote:
> >On Fri, Mar 18, 2016 at 02:07:40PM +0530, akash.goel@intel.com wrote:
> >>+/* emit_store_qword
> >>+ * populate batch buffer with MI_STORE_DWORD_IMM command
> >>+ * @fd: drm file descriptor
> >>+ * @cmd_buf: batch buffer
> >>+ * @dw_offset: write offset in batch buffer
> >>+ * @vaddr: destination Virtual address
> >>+ * @data: u64 data to be stored at destination
> >>+ */
> >>+static int emit_store_qword(int fd, uint32_t *cmd_buf, uint32_t dw_offset,
> >>+			    uint64_t vaddr, uint64_t data)
> >>+{
> >>+	/* Check that softpin addresses are in the correct form */
> >>+	igt_assert_eq_u64(vaddr, igt_canonical_addr(vaddr));
> >>+
> >>+	/* SDI cannot write to unaligned addresses */
> >>+	igt_assert((vaddr & 3) == 0);
> >
> >If I remember correctly a qword write from SDI must be 8 byte aligned.
> >Right?
> 
> Yes right. Sorry, my bad..
> >
> >>+
> >>+	cmd_buf[dw_offset++] = MI_STORE_DWORD_IMM | 0x3;
> >>+	cmd_buf[dw_offset++] = (uint32_t)vaddr;
> >>+	cmd_buf[dw_offset++] = (uint32_t)(vaddr >> 32);
> >>+	cmd_buf[dw_offset++] = data;
> >>+	cmd_buf[dw_offset++] = data >> 32;
> >>+
> >>+	return dw_offset;
> >>+}
> >
> >Hopefully final comments!
> >
> >Missed EINTR handling during evict, If you repeat the busy/hang tests
> >within the igt_fork_signal_helper(); igt_stop_signal_helper() that
> >should cover catching an inopportune signal.
> 
> Fine will add, thanks for suggesting this
> So the signal will interrupt the Driver, which would be waiting for
> the vma unbind to complete, from the eviction path.

Right, and we will report the error back to userspace as EINTR and
userspace will restart the syscall and we expect it to succeed
(eventually). Just useful for flushing out the error handling.

Having just remembered how useful this might be, I just extended
gem_softpin for similar reasons:
+       igt_subtest("evict-active-interruptible") {
+               struct timespec start = {};
+               while (igt_seconds_elapsed(&start) < 20)
+                       test_evict_active(fd);
+       }
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx