Re: [PATCH v4 0/7] Command parser batch buffer copy

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

 



On Fri, Nov 07, 2014 at 02:22:00PM -0800, bradley.d.volkin@xxxxxxxxx wrote:
> From: Brad Volkin <bradley.d.volkin@xxxxxxxxx>
> 
> This is v4 of the series I sent here:
> http://lists.freedesktop.org/archives/intel-gfx/2014-November/054733.html
> 
> This version incorporates most of the feedback from v3. The couple of things
> that I missed (mostly for timing reasons) are:
> * Move 'pending_read_domains |= I915_GEM_DOMAIN_COMMAND' after the parser

Yeah, I think that should still be resolved before merging. I spotted a
few minor things in other patches, too. But besides that I think this is
ready for final review (by someone else, just to diffuse knowledge better)
and then merging.

> * Maybe remove the memsets from the batch copy function

Imo that's part of the larger "don't make the copy suck on vlv" task.
Since vpg cares about that most, I hope it'll happen as a follow-up.

> * Today's feedback from Chris and Daniel r.e. madv

Hm, I think that was just kerneldoc + shrinker integration? Imo with the
reording in the batch pool code to clean up purged buffers we should be
good enough (I hope) wrt shrinking. Kerneldoc as a follow-up would still
be great ofc.

> I'd suggest that the first two could be small follow up patches, and the madv
> changes I did based on Daniel's earlier comments were pulled into a separate
> patch that could be rewritten or modified as needed.

tbh I didn't see anything that looked functionally wrong ... The exec
entry flag is definitely safer&cleaner to make sure we don't have ordering
issues with some other allocation which might steal the buffer before it's
marked active. Also no clues from my mails, so what have you thought of
here?
-Daniel

> 
> Brad Volkin (7):
>   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
>   drm/i915: Mark shadow batch buffers as purgeable
>   drm/i915: Tidy up execbuffer command parsing code
> 
>  Documentation/DocBook/drm.tmpl             |   5 +
>  drivers/gpu/drm/i915/Makefile              |   1 +
>  drivers/gpu/drm/i915/i915_cmd_parser.c     |  97 ++++++++++++++----
>  drivers/gpu/drm/i915/i915_debugfs.c        |  86 ++++++++++++++--
>  drivers/gpu/drm/i915/i915_dma.c            |   1 +
>  drivers/gpu/drm/i915/i915_drv.h            |  24 +++++
>  drivers/gpu/drm/i915/i915_gem.c            |   3 +
>  drivers/gpu/drm/i915/i915_gem_batch_pool.c | 152 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 105 ++++++++++++++++----
>  9 files changed, 430 insertions(+), 44 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_gem_batch_pool.c
> 
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxxxxxxxx
http://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