Re: [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET

2019-11-22 Thread Ville Syrjälä
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 
> Signed-off-by: Matthew Auld 
> Cc: Joonas Lahtinen 
> ---
>  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,
> - 

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET

2019-11-19 Thread Abdiel Janulgue

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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET

2019-11-19 Thread Abdiel Janulgue
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 
Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
---
 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

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET

2019-11-15 Thread Chris Wilson
Quoting Abdiel Janulgue (2019-11-15 11:45:47)
> 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_MMAPDRM_IOWR(DRM_COMMAND_BASE + 
> DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
>  #define DRM_IOCTL_I915_GEM_MMAP_GTTDRM_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

The only question left here is should we

#define I915_MMAP_USE_EXTENSIONS (1 << 8)

from the start and add the dummy user extension handler to ease
adaption.

Otherwise the uABI looks correct; though it merits doing something like

struct drm_i915_gem_mmap_gtt arg = { valid state };
u64 redzone[16];

memset(redzone, 0xc5, sizeof(redzone));
igt_assert_eq(ioctl(I915_GEM_MMAP_GTT, &arg), 0);

as that would have failed with the earlier patch. A useful exercise to
work through to make sure you understand why.

Please write that igt asap.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET

2019-11-15 Thread Chris Wilson
Quoting Abdiel Janulgue (2019-11-15 11:45:47)
> +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,

s/TYPE_GTT/type/?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET

2019-11-15 Thread Abdiel Janulgue
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 
Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/gem/i915_gem_ioctls.h|  4 +-
 drivers/gpu/drm/i915/gem/i915_gem_mman.c  | 42 ---
 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   |  3 +-
 drivers/gpu/drm/i915/i915_drv.h   |  1 -
 include/uapi/drm/i915_drm.h   | 27 
 7 files changed, 72 insertions(+), 9 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 e602dfb44532..d2ed8a463672 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,10 +556,39 @@ __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,
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/i9

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET

2019-11-14 Thread Lucas De Marchi

On Thu, Nov 14, 2019 at 09:09: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


I'm missing the accompanying igt tests for this. The only series I could
find is old, https://patchwork.freedesktop.org/series/65171/, with
pending fixes to do.


Lucas De Marchi




Signed-off-by: Abdiel Janulgue 
Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c  | 44 ++-
.../gpu/drm/i915/gem/i915_gem_object_types.h  |  3 ++
include/uapi/drm/i915_drm.h   | 27 +++-
3 files changed, 71 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 e2df6379c7a7..5c173c29b3d2 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
@@ -535,6 +538,35 @@ __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;
+
+   switch (args->flags) {
+   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, type,
+&args->offset);
+}
+
/**
 * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
 * @dev: DRM device
@@ -554,7 +586,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)
+   return gem_mmap_offset(dev, data, file);
+
+   if (!i915_ggtt_has_aperture(&i915->ggtt))
+   /* 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 632d6b804cfb..07237fbfdaac 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/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5400d7e057f1..e9a80b94 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_PREADDRM_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

[Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET

2019-11-14 Thread Abdiel Janulgue
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

Signed-off-by: Abdiel Janulgue 
Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c  | 44 ++-
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  3 ++
 include/uapi/drm/i915_drm.h   | 27 +++-
 3 files changed, 71 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 e2df6379c7a7..5c173c29b3d2 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
@@ -535,6 +538,35 @@ __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;
+
+   switch (args->flags) {
+   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, type,
+&args->offset);
+}
+
 /**
  * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
  * @dev: DRM device
@@ -554,7 +586,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)
+   return gem_mmap_offset(dev, data, file);
+
+   if (!i915_ggtt_has_aperture(&i915->ggtt))
+   /* 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 632d6b804cfb..07237fbfdaac 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/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5400d7e057f1..e9a80b94 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_MMAPDRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
-#define DRM_IOCTL_I915_GEM_MMAP_GTTDRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt)
+#define DRM_IOCTL_I915_GEM_MMAP_GTTDRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mma