[Intel-gfx] [PATCH v7] drm/i915: Add soft-pinning API for execbuffer

[Tvrtko Ursulin]

On 09/12/15 10:51, Chris Wilson wrote:
> On Wed, Dec 09, 2015 at 10:30:41AM +0000, Tvrtko Ursulin wrote:
>>
>> On 08/12/15 18:49, Michel Thierry wrote:
>>> On 12/8/2015 11:55 AM, Thomas Daniel wrote:
>>>> From: Chris Wilson <chris at chris-wilson.co.uk>
>>>>
>>>> Userspace can pass in an offset that it presumes the object is located
>>>> at. The kernel will then do its utmost to fit the object into that
>>>> location. The assumption is that userspace is handling its own object
>>>> locations (for example along with full-ppgtt) and that the kernel will
>>>> rarely have to make space for the user's requests.
>>>>
>>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>>>
>>>> v2: Fixed incorrect eviction found by Michal Winiarski - fix suggested
>>>> by Chris
>>>> Wilson.  Fixed incorrect error paths causing crash found by Michal
>>>> Winiarski.
>>>> (Not published externally)
>>>>
>>>> v3: Rebased because of trivial conflict in object_bind_to_vm.  Fixed
>>>> eviction
>>>> to allow eviction of soft-pinned objects when another soft-pinned
>>>> object used
>>>> by a subsequent execbuffer overlaps reported by Michal Winiarski.
>>>> (Not published externally)
>>>>
>>>> v4: Moved soft-pinned objects to the front of ordered_vmas so that
>>>> they are
>>>> pinned first after an address conflict happens to avoid repeated
>>>> conflicts in
>>>> rare cases (Suggested by Chris Wilson).  Expanded comment on
>>>> drm_i915_gem_exec_object2.offset to cover this new API.
>>>>
>>>> v5: Added I915_PARAM_HAS_EXEC_SOFTPIN parameter for detecting this
>>>> capability
>>>> (Kristian). Added check for multiple pinnings on eviction (Akash).
>>>> Made sure
>>>> buffers are not considered misplaced without the user specifying
>>>> EXEC_OBJECT_SUPPORTS_48B_ADDRESS.  User must assume responsibility for
>>>> any
>>>> addressing workarounds.  Updated object2.offset field comment again to
>>>> clarify
>>>> NO_RELOC case (Chris).  checkpatch cleanup.
>>>>
>>>> v6: Trivial rebase on latest drm-intel-nightly
>>>>
>>>> v7: Catch attempts to pin above the max virtual address size and return
>>>> EINVAL (Tvrtko). Decouple EXEC_OBJECT_SUPPORTS_48B_ADDRESS and
>>>> EXEC_OBJECT_PINNED flags, user must pass both flags in any attempt to pin
>>>> something at an offset above 4GB (Chris, Daniel Vetter).
>>>>
>>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>>>> Cc: Akash Goel <akash.goel at intel.com>
>>>> Cc: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>>>> Cc: Michal Winiarski <michal.winiarski at intel.com>
>>>> Cc: Zou Nanhai <nanhai.zou at intel.com>
>>>> Cc: Kristian Høgsberg <hoegsberg at gmail.com>
>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
>>>> Signed-off-by: Thomas Daniel <thomas.daniel at intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/i915_dma.c            |  3 ++
>>>>   drivers/gpu/drm/i915/i915_drv.h            |  2 +
>>>>   drivers/gpu/drm/i915/i915_gem.c            | 64
>>>> ++++++++++++++++++++----------
>>>>   drivers/gpu/drm/i915/i915_gem_evict.c      | 39 ++++++++++++++++++
>>>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 16 +++++++-
>>>>   include/uapi/drm/i915_drm.h                | 12 ++++--
>>>>   6 files changed, 111 insertions(+), 25 deletions(-)
>>>>
>>>
>>> Extra support from the other patch aside, v6 already had rb from Akash
>>> and this one,
>>> Reviewed-by: Michel Thierry <michel.thierry at intel.com>
>>
>> This patch was acked by the PDT so I merged it to drm-intel-next-queued.
>
> Please revert immediately. We need to fix the ABI for canonical
> addressing before proceeding. Then please work on the better patch.

Sounds like this is a valid comment, guys please check the thread with 
subject "[PATCH v2] drm/i915: Avoid writing relocs with addresses in 
non-canonical form".

amd64 ABI mandates rules on virtual addresess - 
https://en.wikipedia.org/wiki/X86-64#Canonical_form_addresses.

Regards,

Tvrtko