| Message ID | 20191015083724.24390-3-abdiel.janulgue@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core | expand |
Quoting Abdiel Janulgue (2019-10-15 09:37:22) > This is really just an alias of mmap_gtt. The 'mmap offset' nomenclature > comes 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 > > 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_mman.c | 36 +++++++++++++++++-- > .../gpu/drm/i915/gem/i915_gem_object_types.h | 3 ++ > include/uapi/drm/i915_drm.h | 30 +++++++++++++++- > 3 files changed, 66 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > index 4fb46cbf1bcc..763f3bd78e65 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > @@ -144,6 +144,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 > @@ -171,7 +174,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 > @@ -539,6 +542,27 @@ __assign_gem_object_mmap_data(struct drm_file *file, > return ret; > } > > +static int gem_mmap_offset(struct drm_device *dev, void *data, > + struct drm_file *file) > +{ > + struct drm_i915_gem_mmap_offset *args = data; > + enum i915_mmap_type type; > + > + if ((args->flags & (I915_MMAP_OFFSET_WC | I915_MMAP_OFFSET_UC)) && > + !boot_cpu_has(X86_FEATURE_PAT)) > + return -ENODEV; > + > + if (args->flags & I915_MMAP_OFFSET_WC) > + type = I915_MMAP_TYPE_WC; > + else if (args->flags & I915_MMAP_OFFSET_WB) > + type = I915_MMAP_TYPE_WB; > + else if (args->flags & I915_MMAP_OFFSET_UC) > + type = I915_MMAP_TYPE_UC; else return -EINVAL; for safety and keeping the compiler warnings at bay. > + > + return __assign_gem_object_mmap_data(file, args->handle, type, > + &args->offset); > +} > + > /** > * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing > * @dev: DRM device > @@ -558,7 +582,15 @@ int > i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data, > struct drm_file *file) > { > - struct drm_i915_gem_mmap_gtt *args = data; > + struct drm_i915_gem_mmap_offset *args = data; > + struct drm_i915_private *i915 = to_i915(dev); > + > + if (args->flags & I915_MMAP_OFFSET_FLAGS) > + return gem_mmap_offset(dev, data, file); > + > + if (!HAS_MAPPABLE_APERTURE(i915)) > + /* No aperture, cannot mmap via legacy GTT */ > + return -ENODEV; > > return __assign_gem_object_mmap_data(file, args->handle, > I915_MMAP_TYPE_GTT, > 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 c5c305bd9927..c643d45f5c0d 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > @@ -64,6 +64,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/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 63d40cba97e0..09e777133c81 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -394,7 +394,7 @@ typedef struct _drm_i915_sarea { > #define DRM_IOCTL_I915_GEM_PREAD DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PREAD, struct drm_i915_gem_pread) > #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_GTT 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 +793,34 @@ 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_EXTENSION (1u << 0) > +#define I915_MMAP_OFFSET_WC (1u << 1) > +#define I915_MMAP_OFFSET_WB (1u << 2) > +#define I915_MMAP_OFFSET_UC (1u << 3) Why a set of bits, are the pg_prot not exclusive? > +#define I915_MMAP_OFFSET_FLAGS \ > + (I915_MMAP_OFFSET_EXTENSION | I915_MMAP_OFFSET_WC | \ > + I915_MMAP_OFFSET_WB | I915_MMAP_OFFSET_UC) Why make this part of the uAPI? -Chris
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index 4fb46cbf1bcc..763f3bd78e65 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -144,6 +144,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 @@ -171,7 +174,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 @@ -539,6 +542,27 @@ __assign_gem_object_mmap_data(struct drm_file *file, return ret; } +static int gem_mmap_offset(struct drm_device *dev, void *data, + struct drm_file *file) +{ + struct drm_i915_gem_mmap_offset *args = data; + enum i915_mmap_type type; + + if ((args->flags & (I915_MMAP_OFFSET_WC | I915_MMAP_OFFSET_UC)) && + !boot_cpu_has(X86_FEATURE_PAT)) + return -ENODEV; + + if (args->flags & I915_MMAP_OFFSET_WC) + type = I915_MMAP_TYPE_WC; + else if (args->flags & I915_MMAP_OFFSET_WB) + type = I915_MMAP_TYPE_WB; + else if (args->flags & I915_MMAP_OFFSET_UC) + type = I915_MMAP_TYPE_UC; + + return __assign_gem_object_mmap_data(file, args->handle, type, + &args->offset); +} + /** * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing * @dev: DRM device @@ -558,7 +582,15 @@ int i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data, struct drm_file *file) { - struct drm_i915_gem_mmap_gtt *args = data; + struct drm_i915_gem_mmap_offset *args = data; + struct drm_i915_private *i915 = to_i915(dev); + + if (args->flags & I915_MMAP_OFFSET_FLAGS) + return gem_mmap_offset(dev, data, file); + + if (!HAS_MAPPABLE_APERTURE(i915)) + /* No aperture, cannot mmap via legacy GTT */ + return -ENODEV; return __assign_gem_object_mmap_data(file, args->handle, I915_MMAP_TYPE_GTT, 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 c5c305bd9927..c643d45f5c0d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -64,6 +64,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/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 63d40cba97e0..09e777133c81 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -394,7 +394,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_PREAD DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PREAD, struct drm_i915_gem_pread) #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_GTT 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 +793,34 @@ 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_EXTENSION (1u << 0) +#define I915_MMAP_OFFSET_WC (1u << 1) +#define I915_MMAP_OFFSET_WB (1u << 2) +#define I915_MMAP_OFFSET_UC (1u << 3) +#define I915_MMAP_OFFSET_FLAGS \ + (I915_MMAP_OFFSET_EXTENSION | I915_MMAP_OFFSET_WC | \ + I915_MMAP_OFFSET_WB | I915_MMAP_OFFSET_UC) + __u64 extensions; +}; + struct drm_i915_gem_set_domain { /** Handle for the object */ __u32 handle;