On Wed, Jul 14, 2021 at 12:05 PM Maarten Lankhorst
<maarten.lankhorst@xxxxxxxxxxxxxxx> wrote:
>
> Op 14-07-2021 om 18:17 schreef Jason Ekstrand:
> > On Wed, Jul 14, 2021 at 5:03 AM Maarten Lankhorst
> > <maarten.lankhorst@xxxxxxxxxxxxxxx> wrote:
> >> The FIXED mapping is only used for ttm, and tells userspace that the
> >> mapping type is pre-defined. This disables the other type of mmap
> >> offsets when discrete memory is used, so fix the selftests as well.
> >>
> >> Document the struct as well, so it shows up in docbook.
> >>
> >> Cc: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
> >> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> >> ---
> >> 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 | 46 ++++++++++++++-----
> >> 4 files changed, 77 insertions(+), 14 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..31c4021bb6be 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_FIXED)
> >> + return -ENODEV;
> >> +
> >> *offset = obj->ops->mmap_offset(obj);
> >> return 0;
> >> }
> >>
> >> + if (mmap_type == I915_MMAP_TYPE_FIXED)
> >> + 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_FIXED;
> >> + 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_FIXED:
> >> + type = I915_MMAP_TYPE_FIXED;
> >> + 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_FIXED:
> >> + GEM_WARN_ON(1);
> >> + /* 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 ef3de2ae9723..afbadfc5516b 100644
> >> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> >> @@ -105,6 +105,7 @@ enum i915_mmap_type {
> >> I915_MMAP_TYPE_WC,
> >> I915_MMAP_TYPE_WB,
> >> I915_MMAP_TYPE_UC,
> >> + I915_MMAP_TYPE_FIXED,
> >> };
> >>
> >> 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 1da8bd675e54..52789c8ad337 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_FIXED;
> >> +
> >> + 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_FIXED;
> >> + else if (type == I915_MMAP_TYPE_FIXED)
> >> + 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_FIXED);
> >>
> >> 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_FIXED: return "ttm";
> > fixed?
> >
> > Otherwise, I trolled through the patch and everything looks good. I
> > don't know the code well enough to know if you missed anything but
> > what you have seems sane.
> >
> > Reviewed-by: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
> >
> > I suppose you'll be wanting a couple mesa patches to go with this? I
> > can try and knock something out today.
>
> Yes please! IGT probably needs fixing too. I can probably take a look at that.
As promised: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11888
As usual, we should make sure those are reviewed and we're happy with
them before we land the uAPI but it was easy enough to wire up.
--Jason
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
