Re: [PATCH v3 0/5] Command parser batch buffer copy

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Nov 03, 2014 at 11:19:40AM -0800, bradley.d.volkin@intel.com wrote:
> From: Brad Volkin <bradley.d.volkin@intel.com>
> 
> This is v3 of the series I sent here:
> http://lists.freedesktop.org/archives/intel-gfx/2014-July/048705.html
> 
> Most of the previous commentary still applies. We've fixed the secure
> dispatch regression though, so the series now puts the parser into
> enabling mode in patch 2.
> 
> There are currently some regressions. I've sent i-g-t patches for a couple that
> are test issues. The remaining issues are:
> 
> drv_hangman error-state-capture-*
>     The test has checks that the logged 'gtt_offset' matches the expected
>     offset of the userspace-supplied batch buffer. Similarly for the address
>     in an MI_BATCH_BUFFER_START command found in the logged ringbuffer contents.
>     These obviously won't match if the buffer submitted to hardware is from the
>     batch pool instead of the one from userspace.
> 
> gem_reloc_vs_gpu *-thrash-inactive
> gem_persistent_relocs *-thrash-inactive
>     These fail with this type of error:
> 
>         Test assertion failure function do_test, file gem_reloc_vs_gpu.c:221:
>         Failed assertion: test == 0xdeadbeef
>         mismatch in buffer 0: 0x00000000 instead of 0xdeadbeef
>         child 6 failed with exit status 99
>         Subtest forked-thrash-inactive: FAIL (3.824s)
> 
>     One crashed, apparently in i915_gem_object_move_to_inactive() called via
>     i915_gem_reset(). I assume there's an issue with my active tracking or
>     madv usage for batch pool objects. Any input would be helpful.
> 
> gem_cs_tlb
>     This test takes longer and may time out.
> 
> Brad Volkin (5):
>   drm/i915: Implement a framework for batch buffer pools
>   drm/i915: Use batch pools with the command parser
>   drm/i915: Add a batch pool debugfs file
>   drm/i915: Add batch pool details to i915_gem_objects debugfs
>   drm/i915: Use batch length instead of object size in command parser

Looks good overall, some comments on the 2nd patch. With those addressed
imo ready for detailed review.

Cheers, Daniel
> 
>  Documentation/DocBook/drm.tmpl             |   5 +
>  drivers/gpu/drm/i915/Makefile              |   1 +
>  drivers/gpu/drm/i915/i915_cmd_parser.c     | 102 +++++++++++++++----
>  drivers/gpu/drm/i915/i915_debugfs.c        |  86 ++++++++++++++--
>  drivers/gpu/drm/i915/i915_dma.c            |   1 +
>  drivers/gpu/drm/i915/i915_drv.h            |  26 +++++
>  drivers/gpu/drm/i915/i915_gem.c            |  11 +++
>  drivers/gpu/drm/i915/i915_gem_batch_pool.c | 153 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  62 +++++++++++-
>  9 files changed, 416 insertions(+), 31 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_gem_batch_pool.c
> 
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx