[PATCH] drm/i915: Add I915_PARAM_MMAP_GTT_VERSION to advertise unlimited mmaps

[PATCH] drm/i915: Add I915_PARAM_MMAP_GTT_VERSION to advertise unlimited mmaps

[Chris Wilson]

Now that we have working partial VMA and faulting support for all
objects, including fence support, advertise to userspace that it can
take advantage of unlimited GGTT mmaps.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
 include/uapi/drm/i915_drm.h     | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index fc9273215286..5b2c56777b75 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -355,6 +355,14 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_MIN_EU_IN_POOL:
 		value = INTEL_INFO(dev)->min_eu_in_pool;
 		break;
+	case I915_PARAM_MMAP_GTT_VERSION:
+		/* 0 - Objects have to be smaller than aperture,
+		 *     all simultaneous users have to fit within the
+		 *     available space within the aperture.
+		 * 1 - Objects can any size, and X,Y or untiled
+		 */
+		value = 1;
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5501fe83ed92..03725fe89859 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -387,6 +387,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_EXEC_SOFTPIN	 37
 #define I915_PARAM_HAS_POOLED_EU	 38
 #define I915_PARAM_MIN_EU_IN_POOL	 39
+#define I915_PARAM_MMAP_GTT_VERSION	 40
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] i965: Embrace "unlimited" GTT mmap support

[Chris Wilson]

From about kernel 4.9, GTT mmaps are virtually unlimited. A new
parameter, I915_PARAM_MMAP_GTT_VERSION, is added to advertise the
feature so query it and use it to avoid limiting tiled allocations to
only fit within the mappable aperture.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Kenneth Graunke <kenneth@whitecape.org>
---
 src/mesa/drivers/dri/i965/brw_context.c  | 17 ++---------------
 src/mesa/drivers/dri/i965/brw_context.h  |  2 +-
 src/mesa/drivers/dri/i965/intel_screen.c | 32 ++++++++++++++++++++++++++++++++
 src/mesa/drivers/dri/i965/intel_screen.h |  2 ++
 4 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index b0f9063..79dba1e 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -1033,21 +1033,6 @@ brwCreateContext(gl_api api,
    brw->urb.max_ds_entries = devinfo->urb.max_ds_entries;
    brw->urb.max_gs_entries = devinfo->urb.max_gs_entries;
 
-   /* Estimate the size of the mappable aperture into the GTT.  There's an
-    * ioctl to get the whole GTT size, but not one to get the mappable subset.
-    * It turns out it's basically always 256MB, though some ancient hardware
-    * was smaller.
-    */
-   uint32_t gtt_size = 256 * 1024 * 1024;
-
-   /* We don't want to map two objects such that a memcpy between them would
-    * just fault one mapping in and then the other over and over forever.  So
-    * we would need to divide the GTT size by 2.  Additionally, some GTT is
-    * taken up by things like the framebuffer and the ringbuffer and such, so
-    * be more conservative.
-    */
-   brw->max_gtt_map_object_size = gtt_size / 4;
-
    if (brw->gen == 6)
       brw->urb.gs_present = false;
 
@@ -1058,6 +1043,8 @@ brwCreateContext(gl_api api,
 
    brw->predicate.state = BRW_PREDICATE_STATE_RENDER;
 
+   brw->max_gtt_map_object_size = screen->max_gtt_map_object_size;
+
    brw->use_resource_streamer = screen->has_resource_streamer &&
       (env_var_as_boolean("INTEL_USE_HW_BT", false) ||
        env_var_as_boolean("INTEL_USE_GATHER", false));
diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index f2dd164..523f36c 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -861,7 +861,7 @@ struct brw_context
     */
    bool perf_debug;
 
-   uint32_t max_gtt_map_object_size;
+   uint64_t max_gtt_map_object_size;
 
    int gen;
    int gt;
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 98f1c76..62eacba 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1006,6 +1006,17 @@ intel_get_boolean(struct intel_screen *screen, int param)
    return (intel_get_param(screen, param, &value) == 0) && value;
 }
 
+static int
+intel_get_integer(struct intel_screen *screen, int param)
+{
+   int value = -1;
+
+   if (intel_get_param(screen, param, &value) == 0)
+	   return value;
+
+   return -1;
+}
+
 static void
 intelDestroyScreen(__DRIscreen * sPriv)
 {
@@ -1576,6 +1587,27 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
    if (INTEL_DEBUG & DEBUG_AUB)
       drm_intel_bufmgr_gem_set_aub_dump(intelScreen->bufmgr, true);
 
+#define I915_PARAM_MMAP_GTT_VERSION 40 /* XXX delete me with new libdrm */
+   if (intel_get_integer(intelScreen, I915_PARAM_MMAP_GTT_VERSION) >= 1) {
+      /* Theorectically unlimited! */
+      intelScreen->max_gtt_map_object_size = UINT64_MAX;
+   } else {
+      /* Estimate the size of the mappable aperture into the GTT.  There's an
+       * ioctl to get the whole GTT size, but not one to get the mappable subset.
+       * It turns out it's basically always 256MB, though some ancient hardware
+       * was smaller.
+       */
+      uint32_t gtt_size = 256 * 1024 * 1024;
+
+      /* We don't want to map two objects such that a memcpy between them would
+       * just fault one mapping in and then the other over and over forever.  So
+       * we would need to divide the GTT size by 2.  Additionally, some GTT is
+       * taken up by things like the framebuffer and the ringbuffer and such, so
+       * be more conservative.
+       */
+      intelScreen->max_gtt_map_object_size = gtt_size / 4;
+   }
+
    intelScreen->hw_has_swizzling = intel_detect_swizzling(intelScreen);
    intelScreen->hw_has_timestamp = intel_detect_timestamp(intelScreen);
 
diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
index 35cd5df..fa806a3 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.h
+++ b/src/mesa/drivers/dri/i965/intel_screen.h
@@ -44,6 +44,8 @@ struct intel_screen
 
    __DRIscreen *driScrnPriv;
 
+   uint64_t max_gtt_map_object_size;
+
    bool no_hw;
    bool hw_has_swizzling;
    bool has_fence_fd;
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Add I915_PARAM_MMAP_GTT_VERSION to advertise unlimited mmaps

[Daniel Vetter]

On Wed, Aug 24, 2016 at 08:42:43PM +0100, Chris Wilson wrote:
> Now that we have working partial VMA and faulting support for all
> objects, including fence support, advertise to userspace that it can
> take advantage of unlimited GGTT mmaps.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
>  include/uapi/drm/i915_drm.h     | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index fc9273215286..5b2c56777b75 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -355,6 +355,14 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_MIN_EU_IN_POOL:
>  		value = INTEL_INFO(dev)->min_eu_in_pool;
>  		break;
> +	case I915_PARAM_MMAP_GTT_VERSION:
> +		/* 0 - Objects have to be smaller than aperture,
> +		 *     all simultaneous users have to fit within the
> +		 *     available space within the aperture.
> +		 * 1 - Objects can any size, and X,Y or untiled

can _have_

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> +		 */
> +		value = 1;
> +		break;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 5501fe83ed92..03725fe89859 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -387,6 +387,7 @@ typedef struct drm_i915_irq_wait {
>  #define I915_PARAM_HAS_EXEC_SOFTPIN	 37
>  #define I915_PARAM_HAS_POOLED_EU	 38
>  #define I915_PARAM_MIN_EU_IN_POOL	 39
> +#define I915_PARAM_MMAP_GTT_VERSION	 40
>  
>  typedef struct drm_i915_getparam {
>  	__s32 param;
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Mesa-dev] [PATCH] i965: Embrace "unlimited" GTT mmap support

[Daniel Vetter]

On Wed, Aug 24, 2016 at 08:42:44PM +0100, Chris Wilson wrote:
> From about kernel 4.9, GTT mmaps are virtually unlimited. A new
> parameter, I915_PARAM_MMAP_GTT_VERSION, is added to advertise the
> feature so query it and use it to avoid limiting tiled allocations to
> only fit within the mappable aperture.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenneth@whitecape.org>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  src/mesa/drivers/dri/i965/brw_context.c  | 17 ++---------------
>  src/mesa/drivers/dri/i965/brw_context.h  |  2 +-
>  src/mesa/drivers/dri/i965/intel_screen.c | 32 ++++++++++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/intel_screen.h |  2 ++
>  4 files changed, 37 insertions(+), 16 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index b0f9063..79dba1e 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -1033,21 +1033,6 @@ brwCreateContext(gl_api api,
>     brw->urb.max_ds_entries = devinfo->urb.max_ds_entries;
>     brw->urb.max_gs_entries = devinfo->urb.max_gs_entries;
>  
> -   /* Estimate the size of the mappable aperture into the GTT.  There's an
> -    * ioctl to get the whole GTT size, but not one to get the mappable subset.
> -    * It turns out it's basically always 256MB, though some ancient hardware
> -    * was smaller.
> -    */
> -   uint32_t gtt_size = 256 * 1024 * 1024;
> -
> -   /* We don't want to map two objects such that a memcpy between them would
> -    * just fault one mapping in and then the other over and over forever.  So
> -    * we would need to divide the GTT size by 2.  Additionally, some GTT is
> -    * taken up by things like the framebuffer and the ringbuffer and such, so
> -    * be more conservative.
> -    */
> -   brw->max_gtt_map_object_size = gtt_size / 4;
> -
>     if (brw->gen == 6)
>        brw->urb.gs_present = false;
>  
> @@ -1058,6 +1043,8 @@ brwCreateContext(gl_api api,
>  
>     brw->predicate.state = BRW_PREDICATE_STATE_RENDER;
>  
> +   brw->max_gtt_map_object_size = screen->max_gtt_map_object_size;
> +
>     brw->use_resource_streamer = screen->has_resource_streamer &&
>        (env_var_as_boolean("INTEL_USE_HW_BT", false) ||
>         env_var_as_boolean("INTEL_USE_GATHER", false));
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index f2dd164..523f36c 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -861,7 +861,7 @@ struct brw_context
>      */
>     bool perf_debug;
>  
> -   uint32_t max_gtt_map_object_size;
> +   uint64_t max_gtt_map_object_size;
>  
>     int gen;
>     int gt;
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index 98f1c76..62eacba 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1006,6 +1006,17 @@ intel_get_boolean(struct intel_screen *screen, int param)
>     return (intel_get_param(screen, param, &value) == 0) && value;
>  }
>  
> +static int
> +intel_get_integer(struct intel_screen *screen, int param)
> +{
> +   int value = -1;
> +
> +   if (intel_get_param(screen, param, &value) == 0)
> +	   return value;
> +
> +   return -1;
> +}
> +
>  static void
>  intelDestroyScreen(__DRIscreen * sPriv)
>  {
> @@ -1576,6 +1587,27 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>     if (INTEL_DEBUG & DEBUG_AUB)
>        drm_intel_bufmgr_gem_set_aub_dump(intelScreen->bufmgr, true);
>  
> +#define I915_PARAM_MMAP_GTT_VERSION 40 /* XXX delete me with new libdrm */
> +   if (intel_get_integer(intelScreen, I915_PARAM_MMAP_GTT_VERSION) >= 1) {
> +      /* Theorectically unlimited! */
> +      intelScreen->max_gtt_map_object_size = UINT64_MAX;
> +   } else {
> +      /* Estimate the size of the mappable aperture into the GTT.  There's an
> +       * ioctl to get the whole GTT size, but not one to get the mappable subset.
> +       * It turns out it's basically always 256MB, though some ancient hardware
> +       * was smaller.
> +       */
> +      uint32_t gtt_size = 256 * 1024 * 1024;
> +
> +      /* We don't want to map two objects such that a memcpy between them would
> +       * just fault one mapping in and then the other over and over forever.  So
> +       * we would need to divide the GTT size by 2.  Additionally, some GTT is
> +       * taken up by things like the framebuffer and the ringbuffer and such, so
> +       * be more conservative.
> +       */
> +      intelScreen->max_gtt_map_object_size = gtt_size / 4;
> +   }
> +
>     intelScreen->hw_has_swizzling = intel_detect_swizzling(intelScreen);
>     intelScreen->hw_has_timestamp = intel_detect_timestamp(intelScreen);
>  
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
> index 35cd5df..fa806a3 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.h
> +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> @@ -44,6 +44,8 @@ struct intel_screen
>  
>     __DRIscreen *driScrnPriv;
>  
> +   uint64_t max_gtt_map_object_size;
> +
>     bool no_hw;
>     bool hw_has_swizzling;
>     bool has_fence_fd;
> -- 
> 2.9.3
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] i965: Embrace "unlimited" GTT mmap support

[Kenneth Graunke]

[-- Attachment #1.1: Type: text/plain, Size: 2049 bytes --]

On Wednesday, August 24, 2016 8:42:44 PM PDT Chris Wilson wrote:
> From about kernel 4.9, GTT mmaps are virtually unlimited. A new
> parameter, I915_PARAM_MMAP_GTT_VERSION, is added to advertise the
> feature so query it and use it to avoid limiting tiled allocations to
> only fit within the mappable aperture.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenneth@whitecape.org>

Cool!  I didn't realize we could do page faulting here, since it's
CPU-related.  It's definitely nice to be able to map an unlimited
amount of space, at least as a fallback, even if other methods end
up being more efficient and we prefer those.

[snip]
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index 98f1c76..62eacba 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1006,6 +1006,17 @@ intel_get_boolean(struct intel_screen *screen, int param)
>     return (intel_get_param(screen, param, &value) == 0) && value;
>  }
>  
> +static int
> +intel_get_integer(struct intel_screen *screen, int param)
> +{
> +   int value = -1;
> +
> +   if (intel_get_param(screen, param, &value) == 0)
> +	   return value;

Indentation is a bit off here (spaces only, 3 space indent).

> +
> +   return -1;
> +}
> +
>  static void
>  intelDestroyScreen(__DRIscreen * sPriv)
>  {
> @@ -1576,6 +1587,27 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>     if (INTEL_DEBUG & DEBUG_AUB)
>        drm_intel_bufmgr_gem_set_aub_dump(intelScreen->bufmgr, true);
>  
> +#define I915_PARAM_MMAP_GTT_VERSION 40 /* XXX delete me with new libdrm */

Lately some other developers have been getting grumpy about requiring
new libdrm versions not in their distros.  Maybe for now, we should do:

#ifndef I915_PARAM_MMAP_GTT_VERSION
#define I915_PARAM_MMAP_GTT_VERSION 40 /* XXX delete me with new libdrm */
#endif

and we can leave it for a little while.

Thanks for doing this, Chris!
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH] drm/i915: Add I915_PARAM_MMAP_GTT_VERSION to advertise unlimited mmaps

[Joonas Lahtinen]

On ke, 2016-08-24 at 20:42 +0100, Chris Wilson wrote:
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -355,6 +355,14 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_MIN_EU_IN_POOL:
>  		value = INTEL_INFO(dev)->min_eu_in_pool;
>  		break;
> +	case I915_PARAM_MMAP_GTT_VERSION:
> +		/* 0 - Objects have to be smaller than aperture,
> +		 *     all simultaneous users have to fit within the
> +		 *     available space within the aperture.
> +		 * 1 - Objects can any size, and X,Y or untiled
> +		 */
> +		value = 1;

The actual test would be if this function call succeeds, though.

value = 0 would never be returned, so I'm not sure it'll be useful to
document as such. So I might document as "Failure to execute the query
means..."

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Add I915_PARAM_MMAP_GTT_VERSION to advertise unlimited mmaps

[Chris Wilson]

On Thu, Aug 25, 2016 at 11:00:54AM +0300, Joonas Lahtinen wrote:
> On ke, 2016-08-24 at 20:42 +0100, Chris Wilson wrote:
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -355,6 +355,14 @@ static int i915_getparam(struct drm_device *dev, void *data,
> >  	case I915_PARAM_MIN_EU_IN_POOL:
> >  		value = INTEL_INFO(dev)->min_eu_in_pool;
> >  		break;
> > +	case I915_PARAM_MMAP_GTT_VERSION:
> > +		/* 0 - Objects have to be smaller than aperture,
> > +		 *     all simultaneous users have to fit within the
> > +		 *     available space within the aperture.
> > +		 * 1 - Objects can any size, and X,Y or untiled
> > +		 */
> > +		value = 1;
> 
> The actual test would be if this function call succeeds, though.
> 
> value = 0 would never be returned, so I'm not sure it'll be useful to
> document as such. So I might document as "Failure to execute the query
> means..."

It's also going to get crowded if we put all snippets of ABI information
here.

If I 

#define I915_MMAP_GTT_VERSION 1
/* ... */

at the start of i915_gem_fault() then hopefully we might remember to
update it.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH] drm/i915: Add I915_PARAM_MMAP_GTT_VERSION to advertise unlimited mmaps

[Joonas Lahtinen]

On to, 2016-08-25 at 09:27 +0100, Chris Wilson wrote:
> On Thu, Aug 25, 2016 at 11:00:54AM +0300, Joonas Lahtinen wrote:
> > 
> > On ke, 2016-08-24 at 20:42 +0100, Chris Wilson wrote:
> > > 
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -355,6 +355,14 @@ static int i915_getparam(struct drm_device *dev, void *data,
> > >  	case I915_PARAM_MIN_EU_IN_POOL:
> > >  		value = INTEL_INFO(dev)->min_eu_in_pool;
> > >  		break;
> > > +	case I915_PARAM_MMAP_GTT_VERSION:
> > > +		/* 0 - Objects have to be smaller than aperture,
> > > +		 *     all simultaneous users have to fit within the
> > > +		 *     available space within the aperture.
> > > +		 * 1 - Objects can any size, and X,Y or untiled
> > > +		 */
> > > +		value = 1;
> > The actual test would be if this function call succeeds, though.
> > 
> > value = 0 would never be returned, so I'm not sure it'll be useful to
> > document as such. So I might document as "Failure to execute the query
> > means..."
> It's also going to get crowded if we put all snippets of ABI information
> here.

Yep, but 0 is never returned so the documentation is incorrect as is.
Better clearly describe the that before the parameter was introduced
conditions were as above.

> 
> If I 
> 
> #define I915_MMAP_GTT_VERSION 1
> /* ... */
> 
> at the start of i915_gem_fault() then hopefully we might remember to
> update it.

+1 on this.

Regards, Joonas

> -Chris
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915: Add I915_PARAM_MMAP_GTT_VERSION to advertise unlimited mmaps

[Chris Wilson]

Now that we have working partial VMA and faulting support for all
objects, including fence support, advertise to userspace that it can
take advantage of unlimited GGTT mmaps.

v2: Make room in the kerneldoc for a more detailed explanation of the
limitations of the GTT mmap interface.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c |  7 ++++++
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem.c | 53 +++++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h     |  1 +
 4 files changed, 62 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1d2230f7b749..47fe07283d88 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -355,6 +355,13 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_MIN_EU_IN_POOL:
 		value = INTEL_INFO(dev)->min_eu_in_pool;
 		break;
+	case I915_PARAM_MMAP_GTT_VERSION:
+		/* Though we've started our numbering from 1, and so class all
+		 * earlier versions as 0, in effect their value is undefined as
+		 * the ioctl will report EINVAL for the unknown param!
+		 */
+		value = i915_gem_mmap_gtt_version();
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 04b4fd6c32e4..9fdaf23336df 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3218,6 +3218,7 @@ int i915_gem_dumb_create(struct drm_file *file_priv,
 			 struct drm_mode_create_dumb *args);
 int i915_gem_mmap_gtt(struct drm_file *file_priv, struct drm_device *dev,
 		      uint32_t handle, uint64_t *offset);
+int i915_gem_mmap_gtt_version(void);
 
 void i915_gem_track_fb(struct drm_i915_gem_object *old,
 		       struct drm_i915_gem_object *new,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 04607d4115d6..d37b44126942 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1680,6 +1680,56 @@ static unsigned int tile_row_pages(struct drm_i915_gem_object *obj)
 }
 
 /**
+ * i915_gem_mmap_gtt_version - report the current feature set for GTT mmaps
+ *
+ * A history of the GTT mmap interface:
+ *
+ * 0 - Everything had to fit into the GTT. Both parties of a memcpy had to
+ *     aligned and suitable for fencing, and still fit into the available
+ *     mappable space left by the pinned display objects. A classic problem
+ *     we called the page-fault-of-doom where we would ping-pong between
+ *     two objects that could not fit inside the GTT and so the memcpy
+ *     would page one object in at the expense of the other between every
+ *     single byte.
+ *
+ * 1 - Objects can be any size, and have any compatible fencing (X Y, or none
+ *     as set via i915_gem_set_tiling() [DRM_I915_GEM_SET_TILING]). If the
+ *     object is too large for the available space (or simply too large
+ *     for the mappable aperture!), a view is created instead and faulted
+ *     into userspace. (This view is aligned and sized appropriately for
+ *     fenced access.)
+ *
+ * Restrictions:
+ *
+ *  * snoopable objects cannot be accessed via the GTT. It can cause machine
+ *    hangs on some architectures, corruption on others. An attempt to service
+ *    a GTT page fault from a snoopable object will generate a SIGBUS.
+ *
+ *  * the object must be able to fit into RAM (physical memory, though no
+ *    limited to the mappable aperture).
+ *
+ *
+ * Caveats:
+ *
+ *  * a new GTT page fault will synchronize rendering from the GPU and flush
+ *    all data to system memory. Subsequent access will not be synchronized.
+ *
+ *  * all mappings are revoked on runtime device suspend.
+ *
+ *  * there are only 8, 16 or 32 fence registers to share between all users
+ *    (older machines require fence register for display and blitter access
+ *    as well). Contention of the fence registers will cause the previous users
+ *    to be unmapped and any new access will generate new page faults.
+ *
+ *  * running out of memory while servicing a fault may generate a SIGBUS,
+ *    rather than the expected SIGSEGV.
+ */
+int i915_gem_mmap_gtt_version(void)
+{
+	return 1;
+}
+
+/**
  * i915_gem_fault - fault a page into the GTT
  * @area: CPU VMA in question
  * @vmf: fault info
@@ -1694,6 +1744,9 @@ static unsigned int tile_row_pages(struct drm_i915_gem_object *obj)
  * from the GTT and/or fence registers to make room.  So performance may
  * suffer if the GTT working set is large or there are few fence registers
  * left.
+ *
+ * The current feature set supported by i915_gem_fault() and thus GTT mmaps
+ * is exposed via I915_PARAM_MMAP_GTT_VERSION (see i915_gem_mmap_gtt_version).
  */
 int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
 {
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5501fe83ed92..03725fe89859 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -387,6 +387,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_EXEC_SOFTPIN	 37
 #define I915_PARAM_HAS_POOLED_EU	 38
 #define I915_PARAM_MIN_EU_IN_POOL	 39
+#define I915_PARAM_MMAP_GTT_VERSION	 40
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] i965: Embrace "unlimited" GTT mmap support

[Ian Romanick]

On 08/24/2016 12:42 PM, Chris Wilson wrote:
> From about kernel 4.9, GTT mmaps are virtually unlimited. A new
> parameter, I915_PARAM_MMAP_GTT_VERSION, is added to advertise the
> feature so query it and use it to avoid limiting tiled allocations to
> only fit within the mappable aperture.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_context.c  | 17 ++---------------
>  src/mesa/drivers/dri/i965/brw_context.h  |  2 +-
>  src/mesa/drivers/dri/i965/intel_screen.c | 32 ++++++++++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/intel_screen.h |  2 ++
>  4 files changed, 37 insertions(+), 16 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index b0f9063..79dba1e 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -1033,21 +1033,6 @@ brwCreateContext(gl_api api,
>     brw->urb.max_ds_entries = devinfo->urb.max_ds_entries;
>     brw->urb.max_gs_entries = devinfo->urb.max_gs_entries;
>  
> -   /* Estimate the size of the mappable aperture into the GTT.  There's an
> -    * ioctl to get the whole GTT size, but not one to get the mappable subset.
> -    * It turns out it's basically always 256MB, though some ancient hardware
> -    * was smaller.
> -    */
> -   uint32_t gtt_size = 256 * 1024 * 1024;
> -
> -   /* We don't want to map two objects such that a memcpy between them would
> -    * just fault one mapping in and then the other over and over forever.  So
> -    * we would need to divide the GTT size by 2.  Additionally, some GTT is
> -    * taken up by things like the framebuffer and the ringbuffer and such, so
> -    * be more conservative.
> -    */
> -   brw->max_gtt_map_object_size = gtt_size / 4;
> -
>     if (brw->gen == 6)
>        brw->urb.gs_present = false;
>  
> @@ -1058,6 +1043,8 @@ brwCreateContext(gl_api api,
>  
>     brw->predicate.state = BRW_PREDICATE_STATE_RENDER;
>  
> +   brw->max_gtt_map_object_size = screen->max_gtt_map_object_size;
> +
>     brw->use_resource_streamer = screen->has_resource_streamer &&
>        (env_var_as_boolean("INTEL_USE_HW_BT", false) ||
>         env_var_as_boolean("INTEL_USE_GATHER", false));
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index f2dd164..523f36c 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -861,7 +861,7 @@ struct brw_context
>      */
>     bool perf_debug;
>  
> -   uint32_t max_gtt_map_object_size;
> +   uint64_t max_gtt_map_object_size;
>  
>     int gen;
>     int gt;
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index 98f1c76..62eacba 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1006,6 +1006,17 @@ intel_get_boolean(struct intel_screen *screen, int param)
>     return (intel_get_param(screen, param, &value) == 0) && value;
>  }
>  
> +static int
> +intel_get_integer(struct intel_screen *screen, int param)
> +{
> +   int value = -1;
> +
> +   if (intel_get_param(screen, param, &value) == 0)
> +	   return value;
> +
> +   return -1;
> +}
> +
>  static void
>  intelDestroyScreen(__DRIscreen * sPriv)
>  {
> @@ -1576,6 +1587,27 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>     if (INTEL_DEBUG & DEBUG_AUB)
>        drm_intel_bufmgr_gem_set_aub_dump(intelScreen->bufmgr, true);
>  
> +#define I915_PARAM_MMAP_GTT_VERSION 40 /* XXX delete me with new libdrm */
> +   if (intel_get_integer(intelScreen, I915_PARAM_MMAP_GTT_VERSION) >= 1) {
> +      /* Theorectically unlimited! */
> +      intelScreen->max_gtt_map_object_size = UINT64_MAX;

Well... not quite unlimited, right?  Isn't the actual VMA space less
than 64-bits?  I thought it was more like 48 bits.

> +   } else {
> +      /* Estimate the size of the mappable aperture into the GTT.  There's an
> +       * ioctl to get the whole GTT size, but not one to get the mappable subset.
> +       * It turns out it's basically always 256MB, though some ancient hardware
> +       * was smaller.
> +       */
> +      uint32_t gtt_size = 256 * 1024 * 1024;
> +
> +      /* We don't want to map two objects such that a memcpy between them would
> +       * just fault one mapping in and then the other over and over forever.  So
> +       * we would need to divide the GTT size by 2.  Additionally, some GTT is
> +       * taken up by things like the framebuffer and the ringbuffer and such, so
> +       * be more conservative.
> +       */
> +      intelScreen->max_gtt_map_object_size = gtt_size / 4;
> +   }
> +
>     intelScreen->hw_has_swizzling = intel_detect_swizzling(intelScreen);
>     intelScreen->hw_has_timestamp = intel_detect_timestamp(intelScreen);
>  
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
> index 35cd5df..fa806a3 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.h
> +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> @@ -44,6 +44,8 @@ struct intel_screen
>  
>     __DRIscreen *driScrnPriv;
>  
> +   uint64_t max_gtt_map_object_size;
> +
>     bool no_hw;
>     bool hw_has_swizzling;
>     bool has_fence_fd;
> 

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH] i965: Embrace "unlimited" GTT mmap support

[Chris Wilson]

On Thu, Aug 25, 2016 at 01:43:53PM -0700, Ian Romanick wrote:
> On 08/24/2016 12:42 PM, Chris Wilson wrote:
> > +#define I915_PARAM_MMAP_GTT_VERSION 40 /* XXX delete me with new libdrm */
> > +   if (intel_get_integer(intelScreen, I915_PARAM_MMAP_GTT_VERSION) >= 1) {
> > +      /* Theorectically unlimited! */
> > +      intelScreen->max_gtt_map_object_size = UINT64_MAX;
> 
> Well... not quite unlimited, right?  Isn't the actual VMA space less
> than 64-bits?  I thought it was more like 48 bits.

The object which can be mapped is unconstrained by the GPU's address
space. We are only about 2MiB of the mappable aperture to map a portion
of the object at a time. The limits upon the object size are that we
have a global 64bit space to allocate GTT offsets from shared by all
objects across all clients. And of course the CPU only has a 40bit
address space, and far less physical memory.

Wrt the interface we can claim to be able to support any object up to
64bits in size. You can apply the same argument as for the mappable
aperture and halve/quarter it so that you can fit more than one huge
object at a time - but if you are planning to allocate lots of 60+bit
objects, we will run out of virtual offsets for them rapidly.

A safe value then would be 1<<64 / <typical number of objects>, so say
1<<48. By the time we can allocate more than one 1<<48 object, I
anticipate we will have moved onto 128bit address spaces.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev