Re: [RFC] drm/i915: Add variable gem object size support to i915

[Damien Lespiau <damien.lespiau@intel.com>]

On Mon, Apr 28, 2014 at 04:01:29PM +0100, arun.siluvery@linux.intel.com wrote:
> From: "Siluvery, Arun" <arun.siluvery@intel.com>
> 
> This patch adds support to have gem objects of variable size.
> The size of the gem object obj->size is always constant and this fact
> is tightly coupled in the driver; this implementation allows to vary
> its effective size using an interface similar to fallocate().
> 
> A new ioctl() is introduced to mark a range as scratch/usable.
> Once marked as scratch, associated backing store is released and the
> region is filled with scratch pages. The region can also be unmarked
> at a later point in which case new backing pages are created.
> The range can be anywhere within the object space, it can have multiple
> ranges possibly overlapping forming a large contiguous range.
> 
> There is only one single scratch page and Kernel allows to write to this
> page; userspace need to keep track of scratch page range otherwise any
> subsequent writes to these pages will overwrite previous content.
> 
> This feature is useful where the exact size of the object is not clear
> at the time of its creation, in such case we usually create an object
> with more than the required size but end up using it partially.
> In devices where there are tight memory constraints it would be useful
> to release that additional space which is currently unused. Using this
> interface the region can be simply marked as scratch which releases
> its backing store thus reducing the memory pressure on the kernel.
> 
> Many thanks to Daniel, ChrisW, Tvrtko, Bob for the idea and feedback
> on this implementation.
> 
> v2: fix holes in error handling and use consistent data types (Tvrtko)
>  - If page allocation fails simply return error; do not try to invoke
>    shrinker to free backing store.
>  - Release new pages created by us in case of error during page allocation
>    or sg_table update.
>  - Use 64-bit data types for start and length values to avoid truncation.
> 

(This is not necessarily things one would need to take into account for
this work, just a few thoughts).

One thing I'm wondering is how fitting the "size" parameter really is
when talking about inherently 2D buffers.

For instance, let's take a Y-tiled texture with MIPLAYOUT_RIGHT, if we
want to allocate mip map levels 0 and 1, and use the ioctl "naively" to
reserve the LOD1 region in one go, we'll end up over allocating the
space below LOD1 (if I'm not mistaken that is).

This can be mitigated by several calls to this fallocate ioctl, to
reserve columns of pages (in the case above, columns for the LOD1
region).

So, how about trying to reduce this ioctl overhead by providing a list
of (start, length) in the ioctl structure?

-- 
Damien