[v3,3/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET

Submitted by Abdiel Janulgue on Oct. 15, 2019, 8:37 a.m.

Details

Message ID 20191015083724.24390-3-abdiel.janulgue@linux.intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Abdiel Janulgue Oct. 15, 2019, 8:37 a.m.
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(-)

Patch hide | download patch | download mbox

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;

Comments

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