| Message ID | 20191119113710.1162-2-abdiel.janulgue@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core | expand |
On 19/11/2019 13.37, Abdiel Janulgue wrote: > +struct drm_i915_gem_mmap_offset { > + /** Handle for the object being mapped. */ > + __u32 handle; > + __u32 pad; > + /** > + * Fake offset to use for subsequent mmap call > + * > + * This is a fixed-size type for 32/64 compatibility. > + */ > + __u64 offset; > + > + /** > + * Flags for extended behaviour. > + * > + * It is mandatory that either one of the MMAP_OFFSET flags > + * should be passed here. > + */ > + __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 > + > + __u64 extensions; > +}; The simple memset IGT portion of this, coming up soon. -Abdiel
On Tue, Nov 19, 2019 at 01:37:08PM +0200, Abdiel Janulgue wrote: > This is really just an alias of mmap_gtt. The 'mmap offset' nomenclature > comes from the value returned by this ioctl which is the offset into the > device fd which userpace uses with mmap(2). > > mmap_gtt was our initial mmap_offset implementation, this extends > our CPU mmap support to allow additional fault handlers that depends on > the object's backing pages. > > Note that we multiplex mmap_gtt and mmap_offset through the same ioctl, > and use the zero extending behaviour of drm to differentiate between > them, when we inspect the flags. > > v2: > - Drop the alias, just rename the struct (Chris) > - Don't bail out on no PAT when doing WB mmaps > - Prepare uAPI for further extensions > v3: > - drop MMAP_OFFSET_FLAGS > v4: > - Tweaks, header re-org > > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_ioctls.h | 4 +- > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 45 ++++++++++++++++--- > drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 + > .../gpu/drm/i915/gem/i915_gem_object_types.h | 3 ++ > drivers/gpu/drm/i915/i915_drv.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 1 - > include/uapi/drm/i915_drm.h | 27 +++++++++++ > 7 files changed, 72 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h > index ddc7f2a52b3e..87d8b27f426d 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h > @@ -28,8 +28,8 @@ int i915_gem_madvise_ioctl(struct drm_device *dev, void *data, > struct drm_file *file); > int i915_gem_mmap_ioctl(struct drm_device *dev, void *data, > struct drm_file *file); > -int i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data, > - struct drm_file *file); > +int i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file); > int i915_gem_pread_ioctl(struct drm_device *dev, void *data, > struct drm_file *file); > int i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > index 36fffb671601..bb05c53c03c8 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > @@ -145,6 +145,9 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj) > * 3 - Remove implicit set-domain(GTT) and synchronisation on initial > * pagefault; swapin remains transparent. > * > + * 4 - Support multiple fault handlers per object depending on object's > + * backing storage (a.k.a. MMAP_OFFSET). > + * > * Restrictions: > * > * * snoopable objects cannot be accessed via the GTT. It can cause machine > @@ -172,7 +175,7 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj) > */ > int i915_gem_mmap_gtt_version(void) > { > - return 3; > + return 4; > } > > static inline struct i915_ggtt_view > @@ -538,7 +541,7 @@ __assign_gem_object_mmap_data(struct drm_file *file, > } > > /** > - * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing > + * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing > * @dev: DRM device > * @data: GTT mapping ioctl data > * @file: GEM object info > @@ -553,13 +556,41 @@ __assign_gem_object_mmap_data(struct drm_file *file, > * userspace. > */ > int > -i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data, > - struct drm_file *file) > +i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file) > { > - struct drm_i915_gem_mmap_gtt *args = data; > + struct drm_i915_private *i915 = to_i915(dev); > + struct drm_i915_gem_mmap_offset *args = data; > + enum i915_mmap_type type; > + > + switch (args->flags) { > + case I915_MMAP_OFFSET_GTT: > + if (!i915_ggtt_has_aperture(&i915->ggtt)) > + return -ENODEV; > + type = I915_MMAP_TYPE_GTT; > + break; > + > + case I915_MMAP_OFFSET_WC: > + if (!boot_cpu_has(X86_FEATURE_PAT)) > + return -ENODEV; > + type = I915_MMAP_TYPE_WC; > + break; > + > + case I915_MMAP_OFFSET_WB: > + type = I915_MMAP_TYPE_WB; > + break; > + > + case I915_MMAP_OFFSET_UC: > + if (!boot_cpu_has(X86_FEATURE_PAT)) > + return -ENODEV; > + type = I915_MMAP_TYPE_UC; > + break; > + > + default: > + return -EINVAL; > + } > > - return __assign_gem_object_mmap_data(file, args->handle, > - I915_MMAP_TYPE_GTT, > + return __assign_gem_object_mmap_data(file, args->handle, type, > &args->offset); > } > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h > index 25a3c4d6cd65..4d3b493e853a 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h > @@ -24,5 +24,6 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex) > void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj); > void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj); > void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj); > +int i915_gem_mmap_gtt_version(void); > > #endif > 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 8ff0834a1d5c..c0beb97b56d1 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > @@ -65,6 +65,9 @@ struct drm_i915_gem_object_ops { > > enum i915_mmap_type { > I915_MMAP_TYPE_GTT = 0, > + I915_MMAP_TYPE_WC, > + I915_MMAP_TYPE_WB, > + I915_MMAP_TYPE_UC, > }; > > struct i915_mmap_offset { > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 3e129bcf8a12..ac6d4470ce75 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -2715,7 +2715,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { > DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_RENDER_ALLOW), > + DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_OFFSET, i915_gem_mmap_offset_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling_ioctl, DRM_RENDER_ALLOW), > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a36c2778c09c..b2af6df2eae0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1853,7 +1853,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev) > int i915_gem_dumb_create(struct drm_file *file_priv, > struct drm_device *dev, > struct drm_mode_create_dumb *args); > -int i915_gem_mmap_gtt_version(void); > > int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno); > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 5400d7e057f1..e844c3a8d197 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -395,6 +395,7 @@ typedef struct _drm_i915_sarea { > #define DRM_IOCTL_I915_GEM_PWRITE DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite) > #define DRM_IOCTL_I915_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap) > #define DRM_IOCTL_I915_GEM_MMAP_GTT DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt) > +#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset) > #define DRM_IOCTL_I915_GEM_SET_DOMAIN DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SET_DOMAIN, struct drm_i915_gem_set_domain) > #define DRM_IOCTL_I915_GEM_SW_FINISH DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SW_FINISH, struct drm_i915_gem_sw_finish) > #define DRM_IOCTL_I915_GEM_SET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling) > @@ -793,6 +794,32 @@ struct drm_i915_gem_mmap_gtt { > __u64 offset; > }; > > +struct drm_i915_gem_mmap_offset { > + /** Handle for the object being mapped. */ > + __u32 handle; > + __u32 pad; > + /** > + * Fake offset to use for subsequent mmap call > + * > + * This is a fixed-size type for 32/64 compatibility. > + */ > + __u64 offset; > + > + /** > + * Flags for extended behaviour. > + * > + * It is mandatory that either one of the MMAP_OFFSET flags > + * should be passed here. What does "either one" mean? "at least one", or "one and only one"? > + */ > + __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 These are mean to be used as I915_MMAP_OFFSET_FOO << 0 or 1 << I915_MMAP_OFFSET_FOO ? /me looks at the code. Seems like the <<0 version is correct, and that also explains the earlier question since you can't even pass more than one, nor can you pass none since I915_MMAP_OFFSET_GTT is 0. Maybe these defines should inclue <<0 in them to make the usage 100% obvious? > + > + __u64 extensions; > +}; > + > struct drm_i915_gem_set_domain { > /** Handle for the object */ > __u32 handle; > -- > 2.23.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_ioctls.h b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h index ddc7f2a52b3e..87d8b27f426d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h @@ -28,8 +28,8 @@ int i915_gem_madvise_ioctl(struct drm_device *dev, void *data, struct drm_file *file); int i915_gem_mmap_ioctl(struct drm_device *dev, void *data, struct drm_file *file); -int i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data, - struct drm_file *file); +int i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); int i915_gem_pread_ioctl(struct drm_device *dev, void *data, struct drm_file *file); int i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index 36fffb671601..bb05c53c03c8 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -145,6 +145,9 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj) * 3 - Remove implicit set-domain(GTT) and synchronisation on initial * pagefault; swapin remains transparent. * + * 4 - Support multiple fault handlers per object depending on object's + * backing storage (a.k.a. MMAP_OFFSET). + * * Restrictions: * * * snoopable objects cannot be accessed via the GTT. It can cause machine @@ -172,7 +175,7 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj) */ int i915_gem_mmap_gtt_version(void) { - return 3; + return 4; } static inline struct i915_ggtt_view @@ -538,7 +541,7 @@ __assign_gem_object_mmap_data(struct drm_file *file, } /** - * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing + * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing * @dev: DRM device * @data: GTT mapping ioctl data * @file: GEM object info @@ -553,13 +556,41 @@ __assign_gem_object_mmap_data(struct drm_file *file, * userspace. */ int -i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data, - struct drm_file *file) +i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data, + struct drm_file *file) { - struct drm_i915_gem_mmap_gtt *args = data; + struct drm_i915_private *i915 = to_i915(dev); + struct drm_i915_gem_mmap_offset *args = data; + enum i915_mmap_type type; + + switch (args->flags) { + case I915_MMAP_OFFSET_GTT: + if (!i915_ggtt_has_aperture(&i915->ggtt)) + return -ENODEV; + type = I915_MMAP_TYPE_GTT; + break; + + case I915_MMAP_OFFSET_WC: + if (!boot_cpu_has(X86_FEATURE_PAT)) + return -ENODEV; + type = I915_MMAP_TYPE_WC; + break; + + case I915_MMAP_OFFSET_WB: + type = I915_MMAP_TYPE_WB; + break; + + case I915_MMAP_OFFSET_UC: + if (!boot_cpu_has(X86_FEATURE_PAT)) + return -ENODEV; + type = I915_MMAP_TYPE_UC; + break; + + default: + return -EINVAL; + } - return __assign_gem_object_mmap_data(file, args->handle, - I915_MMAP_TYPE_GTT, + return __assign_gem_object_mmap_data(file, args->handle, type, &args->offset); } diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h index 25a3c4d6cd65..4d3b493e853a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h @@ -24,5 +24,6 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex) void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj); void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj); void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj); +int i915_gem_mmap_gtt_version(void); #endif 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 8ff0834a1d5c..c0beb97b56d1 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -65,6 +65,9 @@ struct drm_i915_gem_object_ops { enum i915_mmap_type { I915_MMAP_TYPE_GTT = 0, + I915_MMAP_TYPE_WC, + I915_MMAP_TYPE_WB, + I915_MMAP_TYPE_UC, }; struct i915_mmap_offset { diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3e129bcf8a12..ac6d4470ce75 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2715,7 +2715,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_OFFSET, i915_gem_mmap_offset_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling_ioctl, DRM_RENDER_ALLOW), diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a36c2778c09c..b2af6df2eae0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1853,7 +1853,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev) int i915_gem_dumb_create(struct drm_file *file_priv, struct drm_device *dev, struct drm_mode_create_dumb *args); -int i915_gem_mmap_gtt_version(void); int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno); diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 5400d7e057f1..e844c3a8d197 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -395,6 +395,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_PWRITE DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite) #define DRM_IOCTL_I915_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap) #define DRM_IOCTL_I915_GEM_MMAP_GTT DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt) +#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset) #define DRM_IOCTL_I915_GEM_SET_DOMAIN DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SET_DOMAIN, struct drm_i915_gem_set_domain) #define DRM_IOCTL_I915_GEM_SW_FINISH DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SW_FINISH, struct drm_i915_gem_sw_finish) #define DRM_IOCTL_I915_GEM_SET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling) @@ -793,6 +794,32 @@ struct drm_i915_gem_mmap_gtt { __u64 offset; }; +struct drm_i915_gem_mmap_offset { + /** Handle for the object being mapped. */ + __u32 handle; + __u32 pad; + /** + * Fake offset to use for subsequent mmap call + * + * This is a fixed-size type for 32/64 compatibility. + */ + __u64 offset; + + /** + * Flags for extended behaviour. + * + * It is mandatory that either one of the MMAP_OFFSET flags + * should be passed here. + */ + __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 + + __u64 extensions; +}; + struct drm_i915_gem_set_domain { /** Handle for the object */ __u32 handle;