Re: [PATCH 05/11] drm/i915: Support for creating Stolen memory backed objects

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	ankitprasad.r.sharma@intel.com, intel-gfx@lists.freedesktop.org,
	akash.goel@intel.com, shashidhar.hiremath@intel.com
Subject: Re: [PATCH 05/11] drm/i915: Support for creating Stolen memory backed objects
Date: Thu, 14 Jan 2016 10:46:39 +0000	[thread overview]
Message-ID: <56977C8F.40304@linux.intel.com> (raw)
In-Reply-To: <20160114102408.GO29867@nuc-i3427.alporthouse.com>

On 14/01/16 10:24, Chris Wilson wrote:
> On Thu, Jan 14, 2016 at 11:46:41AM +0530, ankitprasad.r.sharma@intel.com wrote:
>> From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
>>
>> Extend the drm_i915_gem_create structure to add support for
>> creating Stolen memory backed objects. Added a new flag through
>> which user can specify the preference to allocate the object from
>> stolen memory, which if set, an attempt will be made to allocate
>> the object from stolen memory subject to the availability of
>> free space in the stolen region.
>>
>> v2: Rebased to the latest drm-intel-nightly (Ankit)
>>
>> v3: Changed versioning of GEM_CREATE param, added new comments (Tvrtko)
>>
>> v4: Changed size from 32b to 64b to prevent userspace overflow (Tvrtko)
>> Corrected function arguments ordering (Chris)
>>
>> v5: Corrected function name (Chris)
>>
>> v6: Updated datatype for flags to keep sizeof(drm_i915_gem_create) u64
>> aligned (Chris)
>>
>> v7: Use first 8 bits of gem_create flags for placement (Chris), Add helper
>> function for object allocation from stolen region (Ankit)
>>
>> Testcase: igt/gem_stolen
>>
>> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
>> +	/**
>> +	 * Requested flags (currently used for placement
>> +	 * (which memory domain))
>> +	 *
>> +	 * You can request that the object be created from special memory
>> +	 * rather than regular system pages using this parameter. Such
>> +	 * irregular objects may have certain restrictions (such as CPU
>> +	 * access to a stolen object is verboten).
>> +	 *
>> +	 * This can be used in the future for other purposes too
>> +	 * e.g. specifying tiling/caching/madvise
>> +	 */
>> +	__u64 flags;
>> +#define I915_CREATE_PLACEMENT_NORMAL 	0 /* standard swappable bo  */
>> +#define I915_CREATE_PLACEMENT_STOLEN 	1 /* Cannot use CPU mmaps */
>
> /* Allocate the object from memory reserved for the igfx (stolen).
>   *
>   * Objects allocated from stolen are restricted in the API they can use,
>   * as direct CPU access to stolen memory is prohibited by the system.
>   * This means that you cannot use a regular CPU mmap (either using WB
>   * or with the WC extension). You can still use a GTT mmap, pwrite,
>   * pread and pass it around for use by execbuffer and between processes
>   * like normal.

What happens when a stolen batch buffer gets to relocate_entry_cpu ?

>   *
>   * Stolen memory is a very limited resource and certain functions of the
>   * hardware can only work from within stolen memory. Userspace's
>   * allocations may be evicted from stolen and moved to normal memory as
>   * required. If the allocation is marked as purgeable (using madvise),
>   * the allocation will be dropped and further access to the object's
>   * backing storage will result in -EFAULT. Stolen objects will also be
>   * migrated to normal memory across suspend and resume, as the stolen
>   * memory is not preserved.
>   *
>   * Stolen memory is regarded as a resource placement hint, most suitable
>   * for medium-sized buffers that are only accessed by the GPU and can be
>   * discarded.
>   */

Would it be better if the placement hint did not specifically mention 
stolen memory but specific limitations? Like,

create.flags = NOT_A_BATCHBUFFER | DONT_NEED_MMAP_CPU

?

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx