[Intel-gfx] [PATCH v3 3/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
Chris Wilson
chris at chris-wilson.co.uk
Tue Oct 15 11:14:43 UTC 2019
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 at linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at 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
More information about the Intel-gfx
mailing list