On Fri, Mar 18, 2016 at 03:22:51PM +0530, Goel, Akash wrote:
>
>
> On 3/18/2016 2:52 PM, Chris Wilson wrote:
> >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@xxxxxxxxx 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);
> >+ }
> Thanks I just tested the interruptible versions like this :-
>
> + igt_fork_signal_helper();
> + igt_subtest("evict_active-interruptible")
> + test_evict_active();
The point about looping is to try and ensure that every possible code
path is interrupted (since we only interrupt every 2us and the code paths
tend to be shorter than than!). So we repeat the test in the vain hope
of hitting something else.
> + igt_subtest("evict_hang-interruptible")
> + test_evict_hang();
> + igt_stop_signal_helper();
>
> Actually the hanging object test implicitly exercises the
> interruption case (otherwise the test won't pass), error recovery as
> a part of GPU reset wakes up/interrupts the waiters.
But only in one spot :)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
