| Message ID | 20210701114234.3859716-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Add TTM offset argument to mmap. | expand |
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@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; > 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; > + > default: > return -EINVAL; > } > @@ -968,6 +980,9 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma) > vma->vm_ops = &vm_ops_cpu; > break; > > + case I915_MMAP_TYPE_TTM: > + GEM_WARN_ON(mmo->mmap_type == I915_MMAP_TYPE_TTM); > + /* fall-through */ > case I915_MMAP_TYPE_WB: > vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); > vma->vm_ops = &vm_ops_cpu; > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > index 441f913c87e6..8b872b22a7ec 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > @@ -96,6 +96,7 @@ enum i915_mmap_type { > I915_MMAP_TYPE_WC, > I915_MMAP_TYPE_WB, > I915_MMAP_TYPE_UC, > + I915_MMAP_TYPE_TTM, > }; > > struct i915_mmap_offset { > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > index 607b7d2d4c29..321271bd2fa1 100644 > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > @@ -573,6 +573,14 @@ static int make_obj_busy(struct drm_i915_gem_object *obj) > return 0; > } > > +static enum i915_mmap_type default_mapping(struct drm_i915_private *i915) > +{ > + if (HAS_LMEM(i915)) > + return I915_MMAP_TYPE_TTM; > + > + return I915_MMAP_TYPE_GTT; > +} > + > static bool assert_mmap_offset(struct drm_i915_private *i915, > unsigned long size, > int expected) > @@ -585,7 +593,7 @@ static bool assert_mmap_offset(struct drm_i915_private *i915, > if (IS_ERR(obj)) > return expected && expected == PTR_ERR(obj); > > - ret = __assign_mmap_offset(obj, I915_MMAP_TYPE_GTT, &offset, NULL); > + ret = __assign_mmap_offset(obj, default_mapping(i915), &offset, NULL); > i915_gem_object_put(obj); > > return ret == expected; > @@ -689,7 +697,7 @@ static int igt_mmap_offset_exhaustion(void *arg) > goto out; > } > > - err = __assign_mmap_offset(obj, I915_MMAP_TYPE_GTT, &offset, NULL); > + err = __assign_mmap_offset(obj, default_mapping(i915), &offset, NULL); > if (err) { > pr_err("Unable to insert object into reclaimed hole\n"); > goto err_obj; > @@ -831,8 +839,14 @@ static int wc_check(struct drm_i915_gem_object *obj) > > static bool can_mmap(struct drm_i915_gem_object *obj, enum i915_mmap_type type) > { > + struct drm_i915_private *i915 = to_i915(obj->base.dev); > bool no_map; > > + if (HAS_LMEM(i915)) > + return type == I915_MMAP_TYPE_TTM; > + else if (type == I915_MMAP_TYPE_TTM) > + return false; > + > if (type == I915_MMAP_TYPE_GTT && > !i915_ggtt_has_aperture(&to_i915(obj->base.dev)->ggtt)) > return false; > @@ -970,6 +984,8 @@ static int igt_mmap(void *arg) > err = __igt_mmap(i915, obj, I915_MMAP_TYPE_GTT); > if (err == 0) > err = __igt_mmap(i915, obj, I915_MMAP_TYPE_WC); > + if (err == 0) > + err = __igt_mmap(i915, obj, I915_MMAP_TYPE_TTM); > > i915_gem_object_put(obj); > if (err) > @@ -987,6 +1003,7 @@ static const char *repr_mmap_type(enum i915_mmap_type type) > case I915_MMAP_TYPE_WB: return "wb"; > case I915_MMAP_TYPE_WC: return "wc"; > case I915_MMAP_TYPE_UC: return "uc"; > + case I915_MMAP_TYPE_TTM: return "ttm"; > default: return "unknown"; > } > } > @@ -1100,6 +1117,8 @@ static int igt_mmap_access(void *arg) > err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_WC); > if (err == 0) > err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_UC); > + if (err == 0) > + err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_TTM); > > i915_gem_object_put(obj); > if (err) > @@ -1241,6 +1260,8 @@ static int igt_mmap_gpu(void *arg) > err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_GTT); > if (err == 0) > err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_WC); > + if (err == 0) > + err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_TTM); > > i915_gem_object_put(obj); > if (err) > @@ -1396,6 +1417,8 @@ static int igt_mmap_revoke(void *arg) > err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_GTT); > if (err == 0) > err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC); > + if (err == 0) > + err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_TTM); > > i915_gem_object_put(obj); > if (err) > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 2f70c48567c0..12ec9c7f1711 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -867,10 +867,11 @@ struct drm_i915_gem_mmap_offset { > * (GTT, WC, WB, UC, etc) should be included. > */ > __u64 flags; > -#define I915_MMAP_OFFSET_GTT 0 > -#define I915_MMAP_OFFSET_WC 1 > -#define I915_MMAP_OFFSET_WB 2 > -#define I915_MMAP_OFFSET_UC 3 > +#define I915_MMAP_OFFSET_GTT 0 > +#define I915_MMAP_OFFSET_WC 1 > +#define I915_MMAP_OFFSET_WB 2 > +#define I915_MMAP_OFFSET_UC 3 > +#define I915_MMAP_OFFSET_TTM 4 > > /* > * Zero-terminated chain of extensions.
On Thu, 1 Jul 2021 at 12:50, Maarten Lankhorst <maarten.lankhorst@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@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 > > + > > default: > > return -EINVAL; > > } > > @@ -968,6 +980,9 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma) > > vma->vm_ops = &vm_ops_cpu; > > break; > > > > + case I915_MMAP_TYPE_TTM: > > + GEM_WARN_ON(mmo->mmap_type == I915_MMAP_TYPE_TTM); > > + /* fall-through */ Hmm, we still want WC for lmem, or if it's possible for it to be placed there. > > case I915_MMAP_TYPE_WB: > > vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); > > vma->vm_ops = &vm_ops_cpu; > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > > index 441f913c87e6..8b872b22a7ec 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > > @@ -96,6 +96,7 @@ enum i915_mmap_type { > > I915_MMAP_TYPE_WC, > > I915_MMAP_TYPE_WB, > > I915_MMAP_TYPE_UC, > > + I915_MMAP_TYPE_TTM, > > }; > > > > struct i915_mmap_offset { > > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > > index 607b7d2d4c29..321271bd2fa1 100644 > > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > > @@ -573,6 +573,14 @@ static int make_obj_busy(struct drm_i915_gem_object *obj) > > return 0; > > } > > > > +static enum i915_mmap_type default_mapping(struct drm_i915_private *i915) > > +{ > > + if (HAS_LMEM(i915)) > > + return I915_MMAP_TYPE_TTM; > > + > > + return I915_MMAP_TYPE_GTT; > > +} > > + > > static bool assert_mmap_offset(struct drm_i915_private *i915, > > unsigned long size, > > int expected) > > @@ -585,7 +593,7 @@ static bool assert_mmap_offset(struct drm_i915_private *i915, > > if (IS_ERR(obj)) > > return expected && expected == PTR_ERR(obj); > > > > - ret = __assign_mmap_offset(obj, I915_MMAP_TYPE_GTT, &offset, NULL); > > + ret = __assign_mmap_offset(obj, default_mapping(i915), &offset, NULL); > > i915_gem_object_put(obj); > > > > return ret == expected; > > @@ -689,7 +697,7 @@ static int igt_mmap_offset_exhaustion(void *arg) > > goto out; > > } > > > > - err = __assign_mmap_offset(obj, I915_MMAP_TYPE_GTT, &offset, NULL); > > + err = __assign_mmap_offset(obj, default_mapping(i915), &offset, NULL); > > if (err) { > > pr_err("Unable to insert object into reclaimed hole\n"); > > goto err_obj; > > @@ -831,8 +839,14 @@ static int wc_check(struct drm_i915_gem_object *obj) > > > > static bool can_mmap(struct drm_i915_gem_object *obj, enum i915_mmap_type type) > > { > > + struct drm_i915_private *i915 = to_i915(obj->base.dev); > > bool no_map; > > > > + if (HAS_LMEM(i915)) > > + return type == I915_MMAP_TYPE_TTM; > > + else if (type == I915_MMAP_TYPE_TTM) > > + return false; > > + > > if (type == I915_MMAP_TYPE_GTT && > > !i915_ggtt_has_aperture(&to_i915(obj->base.dev)->ggtt)) > > return false; > > @@ -970,6 +984,8 @@ static int igt_mmap(void *arg) > > err = __igt_mmap(i915, obj, I915_MMAP_TYPE_GTT); > > if (err == 0) > > err = __igt_mmap(i915, obj, I915_MMAP_TYPE_WC); > > + if (err == 0) > > + err = __igt_mmap(i915, obj, I915_MMAP_TYPE_TTM); > > > > i915_gem_object_put(obj); > > if (err) > > @@ -987,6 +1003,7 @@ static const char *repr_mmap_type(enum i915_mmap_type type) > > case I915_MMAP_TYPE_WB: return "wb"; > > case I915_MMAP_TYPE_WC: return "wc"; > > case I915_MMAP_TYPE_UC: return "uc"; > > + case I915_MMAP_TYPE_TTM: return "ttm"; > > default: return "unknown"; > > } > > } > > @@ -1100,6 +1117,8 @@ static int igt_mmap_access(void *arg) > > err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_WC); > > if (err == 0) > > err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_UC); > > + if (err == 0) > > + err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_TTM); > > > > i915_gem_object_put(obj); > > if (err) > > @@ -1241,6 +1260,8 @@ static int igt_mmap_gpu(void *arg) > > err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_GTT); > > if (err == 0) > > err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_WC); > > + if (err == 0) > > + err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_TTM); > > > > i915_gem_object_put(obj); > > if (err) > > @@ -1396,6 +1417,8 @@ static int igt_mmap_revoke(void *arg) > > err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_GTT); > > if (err == 0) > > err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC); > > + if (err == 0) > > + err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_TTM); > > > > i915_gem_object_put(obj); > > if (err) > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > > index 2f70c48567c0..12ec9c7f1711 100644 > > --- a/include/uapi/drm/i915_drm.h > > +++ b/include/uapi/drm/i915_drm.h > > @@ -867,10 +867,11 @@ struct drm_i915_gem_mmap_offset { > > * (GTT, WC, WB, UC, etc) should be included. > > */ > > __u64 flags; > > -#define I915_MMAP_OFFSET_GTT 0 > > -#define I915_MMAP_OFFSET_WC 1 > > -#define I915_MMAP_OFFSET_WB 2 > > -#define I915_MMAP_OFFSET_UC 3 > > +#define I915_MMAP_OFFSET_GTT 0 > > +#define I915_MMAP_OFFSET_WC 1 > > +#define I915_MMAP_OFFSET_WB 2 > > +#define I915_MMAP_OFFSET_UC 3 > > +#define I915_MMAP_OFFSET_TTM 4 > > > > /* > > * Zero-terminated chain of extensions. > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 5 Jul 2021 at 15:36, Matthew Auld <matthew.william.auld@gmail.com> wrote: > > On Thu, 1 Jul 2021 at 12:50, Maarten Lankhorst > <maarten.lankhorst@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@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.
Op 09-07-2021 om 11:30 schreef Matthew Auld: > On Mon, 5 Jul 2021 at 15:36, Matthew Auld > <matthew.william.auld@gmail.com> wrote: >> On Thu, 1 Jul 2021 at 12:50, Maarten Lankhorst >> <maarten.lankhorst@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@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
On Thu, Jul 1, 2021 at 6:42 AM Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote: > > 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@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; > 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; > + > default: > return -EINVAL; > } > @@ -968,6 +980,9 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma) > vma->vm_ops = &vm_ops_cpu; > break; > > + case I915_MMAP_TYPE_TTM: > + GEM_WARN_ON(mmo->mmap_type == I915_MMAP_TYPE_TTM); > + /* fall-through */ > case I915_MMAP_TYPE_WB: > vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); > vma->vm_ops = &vm_ops_cpu; > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > index 441f913c87e6..8b872b22a7ec 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > @@ -96,6 +96,7 @@ enum i915_mmap_type { > I915_MMAP_TYPE_WC, > I915_MMAP_TYPE_WB, > I915_MMAP_TYPE_UC, > + I915_MMAP_TYPE_TTM, > }; > > struct i915_mmap_offset { > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > index 607b7d2d4c29..321271bd2fa1 100644 > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > @@ -573,6 +573,14 @@ static int make_obj_busy(struct drm_i915_gem_object *obj) > return 0; > } > > +static enum i915_mmap_type default_mapping(struct drm_i915_private *i915) > +{ > + if (HAS_LMEM(i915)) > + return I915_MMAP_TYPE_TTM; > + > + return I915_MMAP_TYPE_GTT; > +} > + > static bool assert_mmap_offset(struct drm_i915_private *i915, > unsigned long size, > int expected) > @@ -585,7 +593,7 @@ static bool assert_mmap_offset(struct drm_i915_private *i915, > if (IS_ERR(obj)) > return expected && expected == PTR_ERR(obj); > > - ret = __assign_mmap_offset(obj, I915_MMAP_TYPE_GTT, &offset, NULL); > + ret = __assign_mmap_offset(obj, default_mapping(i915), &offset, NULL); > i915_gem_object_put(obj); > > return ret == expected; > @@ -689,7 +697,7 @@ static int igt_mmap_offset_exhaustion(void *arg) > goto out; > } > > - err = __assign_mmap_offset(obj, I915_MMAP_TYPE_GTT, &offset, NULL); > + err = __assign_mmap_offset(obj, default_mapping(i915), &offset, NULL); > if (err) { > pr_err("Unable to insert object into reclaimed hole\n"); > goto err_obj; > @@ -831,8 +839,14 @@ static int wc_check(struct drm_i915_gem_object *obj) > > static bool can_mmap(struct drm_i915_gem_object *obj, enum i915_mmap_type type) > { > + struct drm_i915_private *i915 = to_i915(obj->base.dev); > bool no_map; > > + if (HAS_LMEM(i915)) > + return type == I915_MMAP_TYPE_TTM; > + else if (type == I915_MMAP_TYPE_TTM) > + return false; > + > if (type == I915_MMAP_TYPE_GTT && > !i915_ggtt_has_aperture(&to_i915(obj->base.dev)->ggtt)) > return false; > @@ -970,6 +984,8 @@ static int igt_mmap(void *arg) > err = __igt_mmap(i915, obj, I915_MMAP_TYPE_GTT); > if (err == 0) > err = __igt_mmap(i915, obj, I915_MMAP_TYPE_WC); > + if (err == 0) > + err = __igt_mmap(i915, obj, I915_MMAP_TYPE_TTM); > > i915_gem_object_put(obj); > if (err) > @@ -987,6 +1003,7 @@ static const char *repr_mmap_type(enum i915_mmap_type type) > case I915_MMAP_TYPE_WB: return "wb"; > case I915_MMAP_TYPE_WC: return "wc"; > case I915_MMAP_TYPE_UC: return "uc"; > + case I915_MMAP_TYPE_TTM: return "ttm"; > default: return "unknown"; > } > } > @@ -1100,6 +1117,8 @@ static int igt_mmap_access(void *arg) > err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_WC); > if (err == 0) > err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_UC); > + if (err == 0) > + err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_TTM); > > i915_gem_object_put(obj); > if (err) > @@ -1241,6 +1260,8 @@ static int igt_mmap_gpu(void *arg) > err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_GTT); > if (err == 0) > err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_WC); > + if (err == 0) > + err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_TTM); > > i915_gem_object_put(obj); > if (err) > @@ -1396,6 +1417,8 @@ static int igt_mmap_revoke(void *arg) > err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_GTT); > if (err == 0) > err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC); > + if (err == 0) > + err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_TTM); > > i915_gem_object_put(obj); > if (err) > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 2f70c48567c0..12ec9c7f1711 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -867,10 +867,11 @@ struct drm_i915_gem_mmap_offset { > * (GTT, WC, WB, UC, etc) should be included. > */ > __u64 flags; > -#define I915_MMAP_OFFSET_GTT 0 > -#define I915_MMAP_OFFSET_WC 1 > -#define I915_MMAP_OFFSET_WB 2 > -#define I915_MMAP_OFFSET_UC 3 > +#define I915_MMAP_OFFSET_GTT 0 > +#define I915_MMAP_OFFSET_WC 1 > +#define I915_MMAP_OFFSET_WB 2 > +#define I915_MMAP_OFFSET_UC 3 > +#define I915_MMAP_OFFSET_TTM 4 I'm not sure I like the name here. TTM is an implementation detail, not a kind of map. I think we want something more like I915_MMAP_OFFSET_IMPLICIT or something like that. The semantics here, as far as I can tell, are "the mmap type is based on the object; you can't change it." On DG1, the mmap type is fixed for all objects. On integrated, we could have a BO create option for the mmap type but we don't yet. --Jason > > /* > * Zero-terminated chain of extensions. > -- > 2.31.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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; 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; + default: return -EINVAL; } @@ -968,6 +980,9 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma) vma->vm_ops = &vm_ops_cpu; break; + case I915_MMAP_TYPE_TTM: + GEM_WARN_ON(mmo->mmap_type == I915_MMAP_TYPE_TTM); + /* fall-through */ case I915_MMAP_TYPE_WB: vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); vma->vm_ops = &vm_ops_cpu; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 441f913c87e6..8b872b22a7ec 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -96,6 +96,7 @@ enum i915_mmap_type { I915_MMAP_TYPE_WC, I915_MMAP_TYPE_WB, I915_MMAP_TYPE_UC, + I915_MMAP_TYPE_TTM, }; struct i915_mmap_offset { diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c index 607b7d2d4c29..321271bd2fa1 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c @@ -573,6 +573,14 @@ static int make_obj_busy(struct drm_i915_gem_object *obj) return 0; } +static enum i915_mmap_type default_mapping(struct drm_i915_private *i915) +{ + if (HAS_LMEM(i915)) + return I915_MMAP_TYPE_TTM; + + return I915_MMAP_TYPE_GTT; +} + static bool assert_mmap_offset(struct drm_i915_private *i915, unsigned long size, int expected) @@ -585,7 +593,7 @@ static bool assert_mmap_offset(struct drm_i915_private *i915, if (IS_ERR(obj)) return expected && expected == PTR_ERR(obj); - ret = __assign_mmap_offset(obj, I915_MMAP_TYPE_GTT, &offset, NULL); + ret = __assign_mmap_offset(obj, default_mapping(i915), &offset, NULL); i915_gem_object_put(obj); return ret == expected; @@ -689,7 +697,7 @@ static int igt_mmap_offset_exhaustion(void *arg) goto out; } - err = __assign_mmap_offset(obj, I915_MMAP_TYPE_GTT, &offset, NULL); + err = __assign_mmap_offset(obj, default_mapping(i915), &offset, NULL); if (err) { pr_err("Unable to insert object into reclaimed hole\n"); goto err_obj; @@ -831,8 +839,14 @@ static int wc_check(struct drm_i915_gem_object *obj) static bool can_mmap(struct drm_i915_gem_object *obj, enum i915_mmap_type type) { + struct drm_i915_private *i915 = to_i915(obj->base.dev); bool no_map; + if (HAS_LMEM(i915)) + return type == I915_MMAP_TYPE_TTM; + else if (type == I915_MMAP_TYPE_TTM) + return false; + if (type == I915_MMAP_TYPE_GTT && !i915_ggtt_has_aperture(&to_i915(obj->base.dev)->ggtt)) return false; @@ -970,6 +984,8 @@ static int igt_mmap(void *arg) err = __igt_mmap(i915, obj, I915_MMAP_TYPE_GTT); if (err == 0) err = __igt_mmap(i915, obj, I915_MMAP_TYPE_WC); + if (err == 0) + err = __igt_mmap(i915, obj, I915_MMAP_TYPE_TTM); i915_gem_object_put(obj); if (err) @@ -987,6 +1003,7 @@ static const char *repr_mmap_type(enum i915_mmap_type type) case I915_MMAP_TYPE_WB: return "wb"; case I915_MMAP_TYPE_WC: return "wc"; case I915_MMAP_TYPE_UC: return "uc"; + case I915_MMAP_TYPE_TTM: return "ttm"; default: return "unknown"; } } @@ -1100,6 +1117,8 @@ static int igt_mmap_access(void *arg) err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_WC); if (err == 0) err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_UC); + if (err == 0) + err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_TTM); i915_gem_object_put(obj); if (err) @@ -1241,6 +1260,8 @@ static int igt_mmap_gpu(void *arg) err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_GTT); if (err == 0) err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_WC); + if (err == 0) + err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_TTM); i915_gem_object_put(obj); if (err) @@ -1396,6 +1417,8 @@ static int igt_mmap_revoke(void *arg) err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_GTT); if (err == 0) err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC); + if (err == 0) + err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_TTM); i915_gem_object_put(obj); if (err) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 2f70c48567c0..12ec9c7f1711 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -867,10 +867,11 @@ struct drm_i915_gem_mmap_offset { * (GTT, WC, WB, UC, etc) should be included. */ __u64 flags; -#define I915_MMAP_OFFSET_GTT 0 -#define I915_MMAP_OFFSET_WC 1 -#define I915_MMAP_OFFSET_WB 2 -#define I915_MMAP_OFFSET_UC 3 +#define I915_MMAP_OFFSET_GTT 0 +#define I915_MMAP_OFFSET_WC 1 +#define I915_MMAP_OFFSET_WB 2 +#define I915_MMAP_OFFSET_UC 3 +#define I915_MMAP_OFFSET_TTM 4 /* * Zero-terminated chain of extensions.
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@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(-)