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

[Deepak S <deepak.s@linux.intel.com>]

On Friday 25 July 2014 02:13 PM, Daniel Vetter wrote:
> 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.

+ Thomas to help us with this.

> 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