[Intel-gfx] [PATCH] drm/i915: Add TTM offset argument to mmap.

[Maarten Lankhorst]

Op 09-07-2021 om 11:30 schreef Matthew Auld:
> On Mon, 5 Jul 2021 at 15:36, Matthew Auld
> <matthew.william.auld at gmail.com> wrote:
>> On Thu, 1 Jul 2021 at 12:50, Maarten Lankhorst
>> <maarten.lankhorst at linux.intel.com> wrote:
>>> Op 01-07-2021 om 13:42 schreef Maarten Lankhorst:
>>>> This is only used for ttm, and tells userspace that the mapping type is
>>>> ignored. This disables the other type of mmap offsets when discrete
>>>> memory is used, so fix the selftests as well.
>>>>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 17 +++++++++++-
>>>>  .../gpu/drm/i915/gem/i915_gem_object_types.h  |  1 +
>>>>  .../drm/i915/gem/selftests/i915_gem_mman.c    | 27 +++++++++++++++++--
>>>>  include/uapi/drm/i915_drm.h                   |  9 ++++---
>>>>  4 files changed, 47 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>>>> index a90f796e85c0..b34be9e5d094 100644
>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>>>> @@ -679,10 +679,16 @@ __assign_mmap_offset(struct drm_i915_gem_object *obj,
>>>>               return -ENODEV;
>>>>
>>>>       if (obj->ops->mmap_offset)  {
>>>> +             if (mmap_type != I915_MMAP_TYPE_TTM)
>>>> +                     return -ENODEV;
>>>> +
>>>>               *offset = obj->ops->mmap_offset(obj);
>>>>               return 0;
>>>>       }
>>>>
>>>> +     if (mmap_type == I915_MMAP_TYPE_TTM)
>>>> +             return -ENODEV;
>>>> +
>>>>       if (mmap_type != I915_MMAP_TYPE_GTT &&
>>>>           !i915_gem_object_has_struct_page(obj) &&
>>>>           !i915_gem_object_has_iomem(obj))
>>>> @@ -727,7 +733,9 @@ i915_gem_dumb_mmap_offset(struct drm_file *file,
>>>>  {
>>>>       enum i915_mmap_type mmap_type;
>>>>
>>>> -     if (boot_cpu_has(X86_FEATURE_PAT))
>>>> +     if (HAS_LMEM(to_i915(dev)))
>>>> +             mmap_type = I915_MMAP_TYPE_TTM;
>>>> +     else if (boot_cpu_has(X86_FEATURE_PAT))
>>>>               mmap_type = I915_MMAP_TYPE_WC;
>> Hmm, I think we still need to check for X86_FEATURE_PAT somewhere,
>> since this is still just WC for lmem?
>>
>>>>       else if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
>>>>               return -ENODEV;
>>>> @@ -798,6 +806,10 @@ i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
>>>>               type = I915_MMAP_TYPE_UC;
>>>>               break;
>>>>
>>>> +     case I915_MMAP_OFFSET_TTM:
>>>> +             type = I915_MMAP_TYPE_TTM;
>>>> +             break;
>> Would it not be simpler to just convert to WC or WB here, if we
>> encounter _TTM? With that we also don't need to touch the dumb mmap
>> stuff and i915_gem_mmap()?
>>
>> if (IS_DGFX()) {
>>         if (args->type != OFFSET_TTM)
>>                return -ENODEV:
>>
>>        GEM_BUG_ON(!obj->mm.n_placements);
>>        if (i915_gem_object_placement_possible(obj, INTEL_MEMORY_LOCAL)) {
>>               type = TYPE_WC;
>>       else
>>               ttype = TYPE_WB;
>> }
>>
>> https://patchwork.freedesktop.org/patch/442579/?series=92209&rev=1
> fyi this patch is now merged, in case you need the
> i915_gem_object_placement_possible() helper.

I think a separate type is better, as it could still be mapped WB if evicted to system memory?

~Maarten