Re: [PATCH 0/4] Introduce a new create ioctl for user specified

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Jul 14, 2014 at 09:53:38AM +0000, Gupta, Sourab wrote:
> On Sun, 2014-07-06 at 18:29 +0530, sourab gupta wrote:
> > On Fri, 2014-06-20 at 10:02 +0000, Gupta, Sourab wrote:
> > > From: Sourab Gupta <sourab.gupta@intel.com>
> > > 
> > > This patch series introduces a new gem create ioctl for user specified
> > > placement.
> > > 
> > > Despite being a unified memory architecture (UMA) some bits of memory
> > > are more equal than others. In particular we have the thorny issue of
> > > stolen memory, memory stolen from the system by the BIOS and reserved
> > > for igfx use. Stolen memory is required for some functions of the GPU
> > > and display engine, but in general it goes wasted. Whilst we cannot
> > > return it back to the system, we need to find some other method for
> > > utilising it. As we do not support direct access to the physical address
> > > in the stolen region, it behaves like a different class of memory,
> > > closer in kin to local GPU memory. This strongly suggests that we need a
> > > placement model like TTM if we are to fully utilize these discrete
> > > chunks of differing memory.
> > >     
> > > This new create ioctl therefore exists to allow the user to create these
> > > second class buffer objects from stolen memory. At the moment direct
> > > access by the CPU through mmaps and pread/pwrite are verboten on the
> > > objects, and so the user must be aware of the limitations of the objects
> > > created. Yet, those limitations rarely reduce the desired functionality
> > > in many use cases and so the user should be able to easily fill the
> > > stolen memory and so help to reduce overall memory pressure.
> > >     
> > > The most obvious use case for stolen memory is for the creation of objects
> > > for the display engine which already have very similar restrictions on
> > > access. However, we want a reasonably general ioctl in order to cater
> > > for diverse scenarios beyond the author's imagination.
> > > 
> > > Chris Wilson (3):
> > >   drm/i915: Clearing buffer objects via blitter engine
> > >   drm/i915: Introduce a new create ioctl for user specified placement
> > >   drm/i915: Add support for stealing purgable stolen pages
> > > 
> > > Deepak S (1):
> > >   drm/i915: Clearing buffer objects via blitter engine for Gen8
> > > 
> > >  drivers/gpu/drm/i915/Makefile          |   1 +
> > >  drivers/gpu/drm/i915/i915_dma.c        |   5 +-
> > >  drivers/gpu/drm/i915/i915_drv.h        |  18 ++-
> > >  drivers/gpu/drm/i915/i915_gem.c        | 208 ++++++++++++++++++++++++++++++---
> > >  drivers/gpu/drm/i915/i915_gem_exec.c   | 139 ++++++++++++++++++++++
> > >  drivers/gpu/drm/i915/i915_gem_stolen.c | 121 +++++++++++++++++--
> > >  drivers/gpu/drm/i915/i915_gem_tiling.c | 106 +++++++++--------
> > >  include/uapi/drm/i915_drm.h            | 107 +++++++++++++++++
> > >  8 files changed, 623 insertions(+), 82 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/i915/i915_gem_exec.c
> > > 
> > 
> > Hi,
> > Can somebody please review this patch series, alongwith the libdrm
> > changes(http://lists.freedesktop.org/archives/intel-gfx/2014-June/047296.html) and igt (http://lists.freedesktop.org/archives/intel-gfx/2014-June/047295.html)
> > 
> > Thanks,
> > Sourab
> 
> Hi,
> Can you please review this patch series.

So on a quick look the kernel side looks sane. The async blitter clear
will have integration issues with the execlist stuff, so having a cpu
clear might be useful and adding the blt clear as a second step. Please
coordinate with the execlist owner.

What's definitely missing is igt coverage. I think we need at least:
- Basic ioctl coverage for create2, including cross-checking with older
  ioctls.
- Testcase for stolen memory including checking that impossible operations
  are all caught correctly.
- Exercising the stolen reaping of purgeable objects.
- Checking that stolen objects are properly cleared.

See http://blog.ffwll.ch/2013/11/testing-requirements-for-drmi915.html for
general testing requirements and
http://blog.ffwll.ch/2013/11/botching-up-ioctls.html for the special
considerations ioctls require.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch