[Mesa-dev] [PATCH 1/9] drm/doc/rfc: i915 DG1 uAPI

2021-04-26 Thread Matthew Auld 
Add an entry for the new uAPI needed for DG1. Also add the overall
upstream plan, including some notes for the TTM conversion.

v2(Daniel):
  - include the overall upstreaming plan
  - add a note for mmap, there are differences here for TTM vs i915
  - bunch of other suggestions from Daniel
v3:
 (Daniel)
  - add a note for set/get caching stuff
  - add some more docs for existing query and extensions stuff
  - add an actual code example for regions query
  - bunch of other stuff
 (Jason)
  - uAPI change(!):
- try a simpler design with the placements extension
- rather than have a generic setparam which can cover multiple
  use cases, have each extension be responsible for one thing
  only
v4:
 (Daniel)
  - add some more notes for ttm conversion
  - bunch of other stuff
 (Jason)
  - uAPI change(!):
- drop all the extra rsvd members for the region_query and
  region_info, just keep the bare minimum needed for padding

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Thomas Hellström 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
Acked-by: Daniel Vetter 
Acked-by: Dave Airlie 
---
 Documentation/gpu/rfc/i915_gem_lmem.h   | 212 
 Documentation/gpu/rfc/i915_gem_lmem.rst | 130 +++
 Documentation/gpu/rfc/index.rst |   4 +
 3 files changed, 346 insertions(+)
 create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
 create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst

diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h 
b/Documentation/gpu/rfc/i915_gem_lmem.h
new file mode 100644
index ..7ed59b6202d5
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_gem_lmem.h
@@ -0,0 +1,212 @@
+/**
+ * enum drm_i915_gem_memory_class - Supported memory classes
+ */
+enum drm_i915_gem_memory_class {
+   /** @I915_MEMORY_CLASS_SYSTEM: System memory */
+   I915_MEMORY_CLASS_SYSTEM = 0,
+   /** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
+   I915_MEMORY_CLASS_DEVICE,
+};
+
+/**
+ * struct drm_i915_gem_memory_class_instance - Identify particular memory 
region
+ */
+struct drm_i915_gem_memory_class_instance {
+   /** @memory_class: See enum drm_i915_gem_memory_class */
+   __u16 memory_class;
+
+   /** @memory_instance: Which instance */
+   __u16 memory_instance;
+};
+
+/**
+ * struct drm_i915_memory_region_info - Describes one region as known to the
+ * driver.
+ *
+ * Note that we reserve some stuff here for potential future work. As an 
example
+ * we might want expose the capabilities(see @caps) for a given region, which
+ * could include things like if the region is CPU mappable/accessible, what are
+ * the supported mapping types etc.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS
+ * at &drm_i915_query_item.query_id.
+ */
+struct drm_i915_memory_region_info {
+   /** @region: The class:instance pair encoding */
+   struct drm_i915_gem_memory_class_instance region;
+
+   /** @pad: MBZ */
+   __u32 pad;
+
+   /** @caps: MBZ */
+   __u64 caps;
+
+   /** @probed_size: Memory probed by the driver (-1 = unknown) */
+   __u64 probed_size;
+
+   /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
+   __u64 unallocated_size;
+};
+
+/**
+ * struct drm_i915_query_memory_regions
+ *
+ * The region info query enumerates all regions known to the driver by filling
+ * in an array of struct drm_i915_memory_region_info structures.
+ *
+ * Example for getting the list of supported regions:
+ *
+ * .. code-block:: C
+ *
+ * struct drm_i915_query_memory_regions *info;
+ * struct drm_i915_query_item item = {
+ * .query_id = DRM_I915_QUERY_MEMORY_REGIONS;
+ * };
+ * struct drm_i915_query query = {
+ * .num_items = 1,
+ * .items_ptr = (uintptr_t)&item,
+ * };
+ * int err, i;
+ *
+ * // First query the size of the blob we need, this needs to be large
+ * // enough to hold our array of regions. The kernel will fill out the
+ * // item.length for us, which is the number of bytes we need.
+ * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
+ * if (err) ...
+ *
+ * info = calloc(1, item.length);
+ * // Now that we allocated the required number of bytes, we call the ioctl
+ * // again, this time with the data_ptr pointing to our newly allocated
+ * // blob, which the kernel can then populate with the all the region 
info.
+ * item.data_ptr = (uintptr_t)&info,
+ *
+ * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
+ * if (err) ...
+ *
+ * // We can now access each region in the array
+ * for (i = 0; i < info->num_regions; i++) {

[Mesa-dev] [PATCH 5/9] drm/i915/uapi: introduce drm_i915_gem_create_ext

2021-04-26 Thread Matthew Auld 
Same old gem_create but with now with extensions support. This is needed
to support various upcoming usecases.

v2:(Chris)
- Use separate ioctl number for gem_create_ext, instead of hijacking
  the existing gem_create ioctl, otherwise we run into the issue
  with being unable to detect if the kernel supports the new extension
  behaviour.
- We now have gem_create_ext.flags, which should be zeroed.
- I915_GEM_CREATE_EXT_SETPARAM value is now zero, since this is the
  index into our array of extensions.
- Setup a "vanilla" object which we can directly apply our extensions
  to.
v3:(Daniel & Jason)
- drop I915_GEM_CREATE_EXT_SETPARAM. Instead just have each extension
  do one thing only, instead of generic setparam which can cover
  various use cases.
- add some kernel-doc.

Signed-off-by: Matthew Auld 
Signed-off-by: CQ Tang 
Cc: Joonas Lahtinen 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c | 56 ++
 drivers/gpu/drm/i915/gem/i915_gem_ioctls.h |  2 +
 drivers/gpu/drm/i915/i915_drv.c|  1 +
 include/uapi/drm/i915_drm.h| 42 
 4 files changed, 101 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c 
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index 73f29224f5fe..90e9eb6601b5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -8,6 +8,7 @@
 
 #include "i915_drv.h"
 #include "i915_trace.h"
+#include "i915_user_extensions.h"
 
 static int i915_gem_publish(struct drm_i915_gem_object *obj,
struct drm_file *file,
@@ -149,3 +150,58 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
i915_gem_object_free(obj);
return ret;
 }
+
+struct create_ext {
+   struct drm_i915_private *i915;
+   struct drm_i915_gem_object *vanilla_object;
+};
+
+static const i915_user_extension_fn create_extensions[] = {
+};
+
+/**
+ * Creates a new mm object and returns a handle to it.
+ * @dev: drm device pointer
+ * @data: ioctl data blob
+ * @file: drm file pointer
+ */
+int
+i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+   struct drm_i915_private *i915 = to_i915(dev);
+   struct drm_i915_gem_create_ext *args = data;
+   struct create_ext ext_data = { .i915 = i915 };
+   struct drm_i915_gem_object *obj;
+   int ret;
+
+   if (args->flags)
+   return -EINVAL;
+
+   i915_gem_flush_free_objects(i915);
+
+   obj = i915_gem_object_alloc();
+   if (!obj)
+   return -ENOMEM;
+
+   ext_data.vanilla_object = obj;
+   ret = i915_user_extensions(u64_to_user_ptr(args->extensions),
+  create_extensions,
+  ARRAY_SIZE(create_extensions),
+  &ext_data);
+   if (ret)
+   goto object_free;
+
+   ret = i915_gem_setup(obj,
+intel_memory_region_by_type(i915,
+INTEL_MEMORY_SYSTEM),
+args->size);
+   if (ret)
+   goto object_free;
+
+   return i915_gem_publish(obj, file, &args->size, &args->handle);
+
+object_free:
+   i915_gem_object_free(obj);
+   return ret;
+}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h 
b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
index 7fd22f3efbef..28d6526e32ab 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
@@ -14,6 +14,8 @@ int i915_gem_busy_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
 int i915_gem_create_ioctl(struct drm_device *dev, void *data,
  struct drm_file *file);
+int i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
 int i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file);
 int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 785dcf20c77b..b5878c089830 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1728,6 +1728,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, drm_noop, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF_DRV(I915_GEM_LEAVEVT, drm_noop, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, 
DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_GEM_CREATE_EXT, i

[Mesa-dev] [PATCH 3/9] drm/i915/query: Expose memory regions through the query uAPI

2021-04-26 Thread Matthew Auld 
From: Abdiel Janulgue 

Returns the available memory region areas supported by the HW.

v2(Daniel & Jason):
- Add some kernel-doc, including example usage.
- Drop all the extra rsvd

Signed-off-by: Abdiel Janulgue 
Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Thomas Hellström 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 drivers/gpu/drm/i915/i915_query.c  |  57 +++
 drivers/gpu/drm/i915/intel_memory_region.h |   8 +-
 include/uapi/drm/i915_drm.h| 109 +
 3 files changed, 169 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_query.c 
b/drivers/gpu/drm/i915/i915_query.c
index fed337ad7b68..0b4cb2e1a15c 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -419,11 +419,68 @@ static int query_perf_config(struct drm_i915_private 
*i915,
}
 }
 
+static int query_memregion_info(struct drm_i915_private *i915,
+   struct drm_i915_query_item *query_item)
+{
+   struct drm_i915_query_memory_regions __user *query_ptr =
+   u64_to_user_ptr(query_item->data_ptr);
+   struct drm_i915_memory_region_info __user *info_ptr =
+   &query_ptr->regions[0];
+   struct drm_i915_memory_region_info info = { };
+   struct drm_i915_query_memory_regions query;
+   struct intel_memory_region *mr;
+   u32 total_length;
+   int ret, id;
+
+   if (query_item->flags != 0)
+   return -EINVAL;
+
+   total_length = sizeof(query);
+   for_each_memory_region(mr, i915, id) {
+   if (mr->private)
+   continue;
+
+   total_length += sizeof(info);
+   }
+
+   ret = copy_query_item(&query, sizeof(query), total_length, query_item);
+   if (ret != 0)
+   return ret;
+
+   if (query.num_regions)
+   return -EINVAL;
+
+   if (query.pad)
+   return  -EINVAL;
+
+   for_each_memory_region(mr, i915, id) {
+   if (mr->private)
+   continue;
+
+   info.region.memory_class = mr->type;
+   info.region.memory_instance = mr->instance;
+   info.probed_size = mr->total;
+   info.unallocated_size = mr->avail;
+
+   if (__copy_to_user(info_ptr, &info, sizeof(info)))
+   return -EFAULT;
+
+   query.num_regions++;
+   info_ptr++;
+   }
+
+   if (__copy_to_user(query_ptr, &query, sizeof(query)))
+   return -EFAULT;
+
+   return total_length;
+}
+
 static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
struct drm_i915_query_item *query_item) 
= {
query_topology_info,
query_engine_info,
query_perf_config,
+   query_memregion_info,
 };
 
 int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
diff --git a/drivers/gpu/drm/i915/intel_memory_region.h 
b/drivers/gpu/drm/i915/intel_memory_region.h
index 942fc4f68764..7cd8e3d66a7f 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.h
+++ b/drivers/gpu/drm/i915/intel_memory_region.h
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "i915_buddy.h"
 
@@ -19,12 +20,9 @@ struct drm_i915_gem_object;
 struct intel_memory_region;
 struct sg_table;
 
-/**
- *  Base memory type
- */
 enum intel_memory_type {
-   INTEL_MEMORY_SYSTEM = 0,
-   INTEL_MEMORY_LOCAL,
+   INTEL_MEMORY_SYSTEM = I915_MEMORY_CLASS_SYSTEM,
+   INTEL_MEMORY_LOCAL = I915_MEMORY_CLASS_DEVICE,
INTEL_MEMORY_STOLEN_SYSTEM,
INTEL_MEMORY_STOLEN_LOCAL,
 };
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6a34243a7646..c5e9c68c310d 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2230,6 +2230,7 @@ struct drm_i915_query_item {
 #define DRM_I915_QUERY_TOPOLOGY_INFO1
 #define DRM_I915_QUERY_ENGINE_INFO 2
 #define DRM_I915_QUERY_PERF_CONFIG  3
+#define DRM_I915_QUERY_MEMORY_REGIONS   4
 /* Must be kept compact -- no holes and well documented */
 
/**
@@ -2464,6 +2465,114 @@ struct drm_i915_query_perf_config {
__u8 data[];
 };
 
+/**
+ * enum drm_i915_gem_memory_class - Supported memory classes
+ */
+enum drm_i915_gem_memory_class {
+   /** @I915_MEMORY_CLASS_SYSTEM: System memory */
+   I915_MEMORY_CLASS_SYSTEM = 0,
+   /** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
+   I915_MEMORY_CLASS_DEVICE,
+};
+
+/**
+ * struct drm_i915_gem_memory_class_instance - Identify particular memory 
region
+ */
+struct drm_i915_gem_memory_class_instance {
+   /** @memory_class: See enum drm_i915_gem_memory_class */
+   __u16 memory_c

[Mesa-dev] [PATCH 4/9] drm/i915: rework gem_create flow for upcoming extensions

2021-04-26 Thread Matthew Auld 
With the upcoming gem_create_ext we want to be able create a "vanilla"
object upfront and pass that directly to the extensions, before actually
initialising the object. Functionally this should be the same expect we
now feed the object into the lower-level region specific init_object.

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Thomas Hellström 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c | 92 +++---
 1 file changed, 65 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c 
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index 45d60e3d98e3..73f29224f5fe 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -7,41 +7,51 @@
 #include "gem/i915_gem_region.h"
 
 #include "i915_drv.h"
+#include "i915_trace.h"
+
+static int i915_gem_publish(struct drm_i915_gem_object *obj,
+   struct drm_file *file,
+   u64 *size_p,
+   u32 *handle_p)
+{
+   u64 size = obj->base.size;
+   int ret;
+
+   ret = drm_gem_handle_create(file, &obj->base, handle_p);
+   /* drop reference from allocate - handle holds it now */
+   i915_gem_object_put(obj);
+   if (ret)
+   return ret;
+
+   *size_p = size;
+   return 0;
+}
 
 static int
-i915_gem_create(struct drm_file *file,
-   struct intel_memory_region *mr,
-   u64 *size_p,
-   u32 *handle_p)
+i915_gem_setup(struct drm_i915_gem_object *obj,
+  struct intel_memory_region *mr,
+  u64 size)
 {
-   struct drm_i915_gem_object *obj;
-   u32 handle;
-   u64 size;
int ret;
 
GEM_BUG_ON(!is_power_of_2(mr->min_page_size));
-   size = round_up(*size_p, mr->min_page_size);
+   size = round_up(size, mr->min_page_size);
if (size == 0)
return -EINVAL;
 
/* For most of the ABI (e.g. mmap) we think in system pages */
GEM_BUG_ON(!IS_ALIGNED(size, PAGE_SIZE));
 
-   /* Allocate the new object */
-   obj = i915_gem_object_create_region(mr, size, 0);
-   if (IS_ERR(obj))
-   return PTR_ERR(obj);
-
-   GEM_BUG_ON(size != obj->base.size);
+   if (i915_gem_object_size_2big(size))
+   return -E2BIG;
 
-   ret = drm_gem_handle_create(file, &obj->base, &handle);
-   /* drop reference from allocate - handle holds it now */
-   i915_gem_object_put(obj);
+   ret = mr->ops->init_object(mr, obj, size, 0);
if (ret)
return ret;
 
-   *handle_p = handle;
-   *size_p = size;
+   GEM_BUG_ON(size != obj->base.size);
+
+   trace_i915_gem_object_create(obj);
return 0;
 }
 
@@ -50,9 +60,11 @@ i915_gem_dumb_create(struct drm_file *file,
 struct drm_device *dev,
 struct drm_mode_create_dumb *args)
 {
+   struct drm_i915_gem_object *obj;
enum intel_memory_type mem_type;
int cpp = DIV_ROUND_UP(args->bpp, 8);
u32 format;
+   int ret;
 
switch (cpp) {
case 1:
@@ -85,10 +97,22 @@ i915_gem_dumb_create(struct drm_file *file,
if (HAS_LMEM(to_i915(dev)))
mem_type = INTEL_MEMORY_LOCAL;
 
-   return i915_gem_create(file,
-  intel_memory_region_by_type(to_i915(dev),
-  mem_type),
-  &args->size, &args->handle);
+   obj = i915_gem_object_alloc();
+   if (!obj)
+   return -ENOMEM;
+
+   ret = i915_gem_setup(obj,
+intel_memory_region_by_type(to_i915(dev),
+ mem_type),
+args->size);
+   if (ret)
+   goto object_free;
+
+   return i915_gem_publish(obj, file, &args->size, &args->handle);
+
+object_free:
+   i915_gem_object_free(obj);
+   return ret;
 }
 
 /**
@@ -103,11 +127,25 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
 {
struct drm_i915_private *i915 = to_i915(dev);
struct drm_i915_gem_create *args = data;
+   struct drm_i915_gem_object *obj;
+   int ret;
 
i915_gem_flush_free_objects(i915);
 
-   return i915_gem_create(file,
-  intel_memory_region_by_type(i915,
-  INTEL_MEMORY_SYSTEM),
-  &args->size, &args->handle);
+   obj = i915_gem_object_alloc();
+   if (!obj)
+   return -ENOMEM;
+
+   ret = i915_gem_setup(obj,
+intel_memory_region_b

[Mesa-dev] [PATCH 6/9] drm/i915/uapi: implement object placement extension

2021-04-26 Thread Matthew Auld 
Add new extension to support setting an immutable-priority-list of
potential placements, at creation time.

If we use the normal gem_create or gem_create_ext without the
extensions/placements then we still get the old behaviour with only
placing the object in system memory.

v2(Daniel & Jason):
- Add a bunch of kernel-doc
- Simplify design for placements extension

Testcase: igt/gem_create/create-ext-placement-sanity-check
Testcase: igt/gem_create/create-ext-placement-each
Testcase: igt/gem_create/create-ext-placement-all
Signed-off-by: Matthew Auld 
Signed-off-by: CQ Tang 
Cc: Joonas Lahtinen 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c| 215 --
 drivers/gpu/drm/i915/gem/i915_gem_object.c|   3 +
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |   6 +
 .../drm/i915/gem/selftests/i915_gem_mman.c|  26 +++
 drivers/gpu/drm/i915/intel_memory_region.c|  16 ++
 drivers/gpu/drm/i915/intel_memory_region.h|   4 +
 include/uapi/drm/i915_drm.h   |  62 +
 7 files changed, 315 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c 
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index 90e9eb6601b5..895f1666a8d3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -4,12 +4,47 @@
  */
 
 #include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_lmem.h"
 #include "gem/i915_gem_region.h"
 
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "i915_user_extensions.h"
 
+static u32 object_max_page_size(struct drm_i915_gem_object *obj)
+{
+   u32 max_page_size = 0;
+   int i;
+
+   for (i = 0; i < obj->mm.n_placements; i++) {
+   struct intel_memory_region *mr = obj->mm.placements[i];
+
+   GEM_BUG_ON(!is_power_of_2(mr->min_page_size));
+   max_page_size = max_t(u32, max_page_size, mr->min_page_size);
+   }
+
+   GEM_BUG_ON(!max_page_size);
+   return max_page_size;
+}
+
+static void object_set_placements(struct drm_i915_gem_object *obj,
+ struct intel_memory_region **placements,
+ unsigned int n_placements)
+{
+   GEM_BUG_ON(!n_placements);
+
+   if (n_placements == 1) {
+   struct intel_memory_region *mr = placements[0];
+   struct drm_i915_private *i915 = mr->i915;
+
+   obj->mm.placements = &i915->mm.regions[mr->id];
+   obj->mm.n_placements = 1;
+   } else {
+   obj->mm.placements = placements;
+   obj->mm.n_placements = n_placements;
+   }
+}
+
 static int i915_gem_publish(struct drm_i915_gem_object *obj,
struct drm_file *file,
u64 *size_p,
@@ -29,14 +64,12 @@ static int i915_gem_publish(struct drm_i915_gem_object *obj,
 }
 
 static int
-i915_gem_setup(struct drm_i915_gem_object *obj,
-  struct intel_memory_region *mr,
-  u64 size)
+i915_gem_setup(struct drm_i915_gem_object *obj, u64 size)
 {
+   struct intel_memory_region *mr = obj->mm.placements[0];
int ret;
 
-   GEM_BUG_ON(!is_power_of_2(mr->min_page_size));
-   size = round_up(size, mr->min_page_size);
+   size = round_up(size, object_max_page_size(obj));
if (size == 0)
return -EINVAL;
 
@@ -53,6 +86,7 @@ i915_gem_setup(struct drm_i915_gem_object *obj,
GEM_BUG_ON(size != obj->base.size);
 
trace_i915_gem_object_create(obj);
+
return 0;
 }
 
@@ -62,6 +96,7 @@ i915_gem_dumb_create(struct drm_file *file,
 struct drm_mode_create_dumb *args)
 {
struct drm_i915_gem_object *obj;
+   struct intel_memory_region *mr;
enum intel_memory_type mem_type;
int cpp = DIV_ROUND_UP(args->bpp, 8);
u32 format;
@@ -102,10 +137,10 @@ i915_gem_dumb_create(struct drm_file *file,
if (!obj)
return -ENOMEM;
 
-   ret = i915_gem_setup(obj,
-intel_memory_region_by_type(to_i915(dev),
- mem_type),
-args->size);
+   mr = intel_memory_region_by_type(to_i915(dev), mem_type);
+   object_set_placements(obj, &mr, 1);
+
+   ret = i915_gem_setup(obj, args->size);
if (ret)
goto object_free;
 
@@ -129,6 +164,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
struct drm_i915_private *i915 = to_i915(dev);
struct drm_i915_gem_create *args = data;
struct drm_i915_gem_object *obj;
+   struct intel_memory_region *mr;
int ret;
 
i915_gem_flush_free_objects(i915);
@@ -137,10 +173,10 @@ i915_gem_create_ioct

[Mesa-dev] [PATCH 2/9] drm/i915: mark stolen as private

2021-04-26 Thread Matthew Auld 
In the next patch we want to expose the supported regions to userspace,
which can then be fed into the gem_create_ext placement extensions. For
now treat stolen memory as private from userspace pov.

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Thomas Hellström 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 4 
 drivers/gpu/drm/i915/intel_memory_region.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index c5b64b2400e8..3bcbb146511a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -803,6 +803,8 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915)
 
intel_memory_region_set_name(mem, "stolen-local");
 
+   mem->private = true;
+
return mem;
 }
 
@@ -821,6 +823,8 @@ i915_gem_stolen_smem_setup(struct drm_i915_private *i915)
 
intel_memory_region_set_name(mem, "stolen-system");
 
+   mem->private = true;
+
return mem;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_memory_region.h 
b/drivers/gpu/drm/i915/intel_memory_region.h
index 4c8ec15af55f..942fc4f68764 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.h
+++ b/drivers/gpu/drm/i915/intel_memory_region.h
@@ -86,6 +86,7 @@ struct intel_memory_region {
u16 instance;
enum intel_region_id id;
char name[16];
+   bool private; /* not for userspace */
 
struct list_head reserved;
 
-- 
2.26.3

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

[Mesa-dev] [PATCH 7/9] drm/i915/lmem: support optional CPU clearing for special internal use

2021-04-26 Thread Matthew Auld 
For some internal device local-memory objects it would be useful to have
an option to CPU clear the pages upon gathering the backing store. Note
that this might be before the blitter is useable, which is the case for
some internal GuC objects.

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Thomas Hellström 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  8 +-
 drivers/gpu/drm/i915/gem/i915_gem_region.c| 22 +
 .../drm/i915/selftests/intel_memory_region.c  | 87 ++-
 3 files changed, 113 insertions(+), 4 deletions(-)

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 69d6e54bc569..0727d0c76aa0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -172,11 +172,13 @@ struct drm_i915_gem_object {
 #define I915_BO_ALLOC_CONTIGUOUS BIT(0)
 #define I915_BO_ALLOC_VOLATILE   BIT(1)
 #define I915_BO_ALLOC_STRUCT_PAGE BIT(2)
+#define I915_BO_ALLOC_CPU_CLEAR  BIT(3)
 #define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | \
 I915_BO_ALLOC_VOLATILE | \
-I915_BO_ALLOC_STRUCT_PAGE)
-#define I915_BO_READONLY BIT(3)
-#define I915_TILING_QUIRK_BIT4 /* unknown swizzling; do not release! */
+I915_BO_ALLOC_STRUCT_PAGE | \
+I915_BO_ALLOC_CPU_CLEAR)
+#define I915_BO_READONLY BIT(4)
+#define I915_TILING_QUIRK_BIT5 /* unknown swizzling; do not release! */
 
/*
 * Is the object to be mapped as read-only to the GPU
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.c 
b/drivers/gpu/drm/i915/gem/i915_gem_region.c
index 6a84fb6dde24..5d603098da57 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_region.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_region.c
@@ -95,6 +95,28 @@ i915_gem_object_get_pages_buddy(struct drm_i915_gem_object 
*obj)
sg_mark_end(sg);
i915_sg_trim(st);
 
+   /* Intended for kernel internal use only */
+   if (obj->flags & I915_BO_ALLOC_CPU_CLEAR) {
+   struct scatterlist *sg;
+   unsigned long i;
+
+   for_each_sg(st->sgl, sg, st->nents, i) {
+   unsigned int length;
+   void __iomem *vaddr;
+   dma_addr_t daddr;
+
+   daddr = sg_dma_address(sg);
+   daddr -= mem->region.start;
+   length = sg_dma_len(sg);
+
+   vaddr = io_mapping_map_wc(&mem->iomap, daddr, length);
+   memset64(vaddr, 0, length / sizeof(u64));
+   io_mapping_unmap(vaddr);
+   }
+
+   wmb();
+   }
+
__i915_gem_object_set_pages(obj, st, sg_page_sizes);
 
return 0;
diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c 
b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
index a5fc0bf3feb9..0fe4c81f7589 100644
--- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
@@ -513,7 +513,7 @@ static int igt_cpu_check(struct drm_i915_gem_object *obj, 
u32 dword, u32 val)
if (err)
return err;
 
-   ptr = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WC);
+   ptr = i915_gem_object_pin_map(obj, I915_MAP_WC);
if (IS_ERR(ptr))
return PTR_ERR(ptr);
 
@@ -593,7 +593,9 @@ static int igt_gpu_write(struct i915_gem_context *ctx,
if (err)
break;
 
+   i915_gem_object_lock(obj, NULL);
err = igt_cpu_check(obj, dword, rng);
+   i915_gem_object_unlock(obj);
if (err)
break;
} while (!__igt_timeout(end_time, NULL));
@@ -629,6 +631,88 @@ static int igt_lmem_create(void *arg)
return err;
 }
 
+static int igt_lmem_create_cleared_cpu(void *arg)
+{
+   struct drm_i915_private *i915 = arg;
+   I915_RND_STATE(prng);
+   IGT_TIMEOUT(end_time);
+   u32 size, i;
+   int err;
+
+   i915_gem_drain_freed_objects(i915);
+
+   size = max_t(u32, PAGE_SIZE, i915_prandom_u32_max_state(SZ_32M, &prng));
+   size = round_up(size, PAGE_SIZE);
+   i = 0;
+
+   do {
+   struct drm_i915_gem_object *obj;
+   void __iomem *vaddr;
+   unsigned int flags;
+   u32 dword, val;
+
+   /*
+* Alternate between cleared and uncleared allocations, while
+* also dirtying the pages each time to check that the pages are
+* always cleared if requested, since we should get some over

[Mesa-dev] [PATCH 8/9] drm/i915/gem: clear userspace buffers for LMEM

2021-04-26 Thread Matthew Auld 
All userspace objects must be cleared when allocating the backing store,
before they are potentially visible to userspace.  For now use simple
CPU based clearing to do this for device local-memory objects, note that
in the near future this will instead use the blitter engine.

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Thomas Hellström 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c 
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index 895f1666a8d3..338f3883e238 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -67,6 +67,7 @@ static int
 i915_gem_setup(struct drm_i915_gem_object *obj, u64 size)
 {
struct intel_memory_region *mr = obj->mm.placements[0];
+   unsigned int flags;
int ret;
 
size = round_up(size, object_max_page_size(obj));
@@ -79,7 +80,16 @@ i915_gem_setup(struct drm_i915_gem_object *obj, u64 size)
if (i915_gem_object_size_2big(size))
return -E2BIG;
 
-   ret = mr->ops->init_object(mr, obj, size, 0);
+   /*
+* For now resort to CPU based clearing for device local-memory, in the
+* near future this will use the blitter engine for accelerated, GPU
+* based clearing.
+*/
+   flags = 0;
+   if (mr->type == INTEL_MEMORY_LOCAL)
+   flags = I915_BO_ALLOC_CPU_CLEAR;
+
+   ret = mr->ops->init_object(mr, obj, size, flags);
if (ret)
return ret;
 
-- 
2.26.3

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

[Mesa-dev] [PATCH 9/9] drm/i915/gem: hide new uAPI behind CONFIG_BROKEN

2021-04-26 Thread Matthew Auld 
Treat it the same as the fake local-memory stuff, where it is disabled
for normal kernels, in case some random UMD is tempted to use this. Once
we have all the other bits and pieces in place, like the TTM conversion,
we can turn this on for real.

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Thomas Hellström 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c | 3 +++
 drivers/gpu/drm/i915/i915_query.c  | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c 
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index 338f3883e238..1d0728b878d5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -332,6 +332,9 @@ static int ext_set_placements(struct i915_user_extension 
__user *base,
 {
struct drm_i915_gem_create_ext_memory_regions ext;
 
+   if (!IS_ENABLED(CONFIG_DRM_I915_UNSTABLE_FAKE_LMEM))
+   return -ENODEV;
+
if (copy_from_user(&ext, base, sizeof(ext)))
return -EFAULT;
 
diff --git a/drivers/gpu/drm/i915/i915_query.c 
b/drivers/gpu/drm/i915/i915_query.c
index 0b4cb2e1a15c..561684ded4a0 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -432,6 +432,9 @@ static int query_memregion_info(struct drm_i915_private 
*i915,
u32 total_length;
int ret, id;
 
+   if (!IS_ENABLED(CONFIG_DRM_I915_UNSTABLE_FAKE_LMEM))
+   return -ENODEV;
+
if (query_item->flags != 0)
return -EINVAL;
 
-- 
2.26.3

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

Re: [Mesa-dev] [PATCH 1/9] drm/doc/rfc: i915 DG1 uAPI

2021-04-26 Thread Jason Ekstrand 
On Mon, Apr 26, 2021 at 4:42 AM Matthew Auld  wrote:
>
> Add an entry for the new uAPI needed for DG1. Also add the overall
> upstream plan, including some notes for the TTM conversion.
>
> v2(Daniel):
>   - include the overall upstreaming plan
>   - add a note for mmap, there are differences here for TTM vs i915
>   - bunch of other suggestions from Daniel
> v3:
>  (Daniel)
>   - add a note for set/get caching stuff
>   - add some more docs for existing query and extensions stuff
>   - add an actual code example for regions query
>   - bunch of other stuff
>  (Jason)
>   - uAPI change(!):
> - try a simpler design with the placements extension
> - rather than have a generic setparam which can cover multiple
>   use cases, have each extension be responsible for one thing
>   only
> v4:
>  (Daniel)
>   - add some more notes for ttm conversion
>   - bunch of other stuff
>  (Jason)
>   - uAPI change(!):
> - drop all the extra rsvd members for the region_query and
>   region_info, just keep the bare minimum needed for padding
>
> Signed-off-by: Matthew Auld 
> Cc: Joonas Lahtinen 
> Cc: Thomas Hellström 
> Cc: Daniele Ceraolo Spurio 
> Cc: Lionel Landwerlin 
> Cc: Jon Bloomfield 
> Cc: Jordan Justen 
> Cc: Daniel Vetter 
> Cc: Kenneth Graunke 
> Cc: Jason Ekstrand 
> Cc: Dave Airlie 
> Cc: dri-de...@lists.freedesktop.org
> Cc: mesa-dev@lists.freedesktop.org
> Acked-by: Daniel Vetter 
> Acked-by: Dave Airlie 
> ---
>  Documentation/gpu/rfc/i915_gem_lmem.h   | 212 
>  Documentation/gpu/rfc/i915_gem_lmem.rst | 130 +++
>  Documentation/gpu/rfc/index.rst |   4 +
>  3 files changed, 346 insertions(+)
>  create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
>  create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst
>
> diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h 
> b/Documentation/gpu/rfc/i915_gem_lmem.h
> new file mode 100644
> index ..7ed59b6202d5
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_gem_lmem.h
> @@ -0,0 +1,212 @@
> +/**
> + * enum drm_i915_gem_memory_class - Supported memory classes
> + */
> +enum drm_i915_gem_memory_class {
> +   /** @I915_MEMORY_CLASS_SYSTEM: System memory */
> +   I915_MEMORY_CLASS_SYSTEM = 0,
> +   /** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
> +   I915_MEMORY_CLASS_DEVICE,
> +};
> +
> +/**
> + * struct drm_i915_gem_memory_class_instance - Identify particular memory 
> region
> + */
> +struct drm_i915_gem_memory_class_instance {
> +   /** @memory_class: See enum drm_i915_gem_memory_class */
> +   __u16 memory_class;
> +
> +   /** @memory_instance: Which instance */
> +   __u16 memory_instance;
> +};
> +
> +/**
> + * struct drm_i915_memory_region_info - Describes one region as known to the
> + * driver.
> + *
> + * Note that we reserve some stuff here for potential future work. As an 
> example
> + * we might want expose the capabilities(see @caps) for a given region, which
> + * could include things like if the region is CPU mappable/accessible, what 
> are
> + * the supported mapping types etc.
> + *
> + * Note this is using both struct drm_i915_query_item and struct 
> drm_i915_query.
> + * For this new query we are adding the new query id 
> DRM_I915_QUERY_MEMORY_REGIONS
> + * at &drm_i915_query_item.query_id.
> + */
> +struct drm_i915_memory_region_info {
> +   /** @region: The class:instance pair encoding */
> +   struct drm_i915_gem_memory_class_instance region;
> +
> +   /** @pad: MBZ */
> +   __u32 pad;
> +
> +   /** @caps: MBZ */
> +   __u64 caps;
> +
> +   /** @probed_size: Memory probed by the driver (-1 = unknown) */
> +   __u64 probed_size;
> +
> +   /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
> +   __u64 unallocated_size;
> +};
> +
> +/**
> + * struct drm_i915_query_memory_regions
> + *
> + * The region info query enumerates all regions known to the driver by 
> filling
> + * in an array of struct drm_i915_memory_region_info structures.
> + *
> + * Example for getting the list of supported regions:
> + *
> + * .. code-block:: C
> + *
> + * struct drm_i915_query_memory_regions *info;
> + * struct drm_i915_query_item item = {
> + * .query_id = DRM_I915_QUERY_MEMORY_REGIONS;
> + * };
> + * struct drm_i915_query query = {
> + * .num_items = 1,
> + * .items_ptr = (uintptr_t)&item,
> + * };
> + * int err, i;
> + *
> + * // First query the size of the blob we need, this needs to be large
> + * // enough to hold our array of regions. The kernel will fill out the
> + * // item.length for us, which is the number of bytes we need.
> + * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
> + * if (err) ...
> + *
> + * info = calloc(1, item.length);
> + * // Now that we allocated the required number of bytes, we call the 
> ioctl
> + * // again, this time with the data_ptr

Re: [Mesa-dev] [PATCH v3 4/4] drm/doc/rfc: i915 DG1 uAPI

2021-04-26 Thread Jason Ekstrand 
On Wed, Apr 21, 2021 at 2:23 PM Daniel Vetter  wrote:
>
> On Wed, Apr 21, 2021 at 8:28 PM Tvrtko Ursulin
>  wrote:
> > On 21/04/2021 18:17, Jason Ekstrand wrote:
> > > On Wed, Apr 21, 2021 at 9:25 AM Tvrtko Ursulin
> > >  wrote:
> > >> On 21/04/2021 14:54, Jason Ekstrand wrote:
> > >>> On Wed, Apr 21, 2021 at 3:22 AM Tvrtko Ursulin
> > >>>  wrote:
> >  On 20/04/2021 18:00, Jason Ekstrand wrote:
> >  I am not claiming to know memory region query will end up the same, and
> >  I definitely agree we cannot guess the future. I am just saying rsvd
> >  fields are inconsequential really in terms of maintenance burden and
> >  have been proven useful in the past. So I disagree with the drive to
> >  kick them all out.
> > >>>
> > >>> Sure, it doesn't cost anything to have extra zeros in the struct.
> > >>> However, if/when the API grows using rsvd fields, we end up with "if
> > >>> CAP_FOO is set, rsvd[5] means blah" which makes for a horribly
> > >>> confusing API.  As a userspace person who has to remember how to use
> > >>> this stuff, I'd rather make another call or chain in a struct than try
> > >>> to remember and/or figure out what all 8 rsvd fields mean.
> > >>
> > >> Well it's not called rsvd in the uapi which is aware of the new field
> > >> but has a new name.
> > >
> > > Are we allowed to do that?  This is a genuine question.  When I've
> > > tried in the past (cliprects), I was told we couldn't rename it even
> > > though literally no one had used it in code for years.
> >
> > Well we did the union for pad_to_size so I thought we are allowed that
> > trick at least. From my experience backward source level compatibility
> > is not always there even with things like glibc. Despite that, are we
> > generally required to stay backward source compatible I will not claim
> > either way.

I'm starting to lose the will to care about this particular bike shed.
I think I'm fine with keeping some RSVD fields as long as:

 1. We're very clear in the docs with flags and caps about what things
are inputs and what things are outputs and how they interact.
Preferably, everything is an output.
 2. If we already know that caps is useless without supported_caps,
let's either add supported_caps in now or throw out both and use some
rsvd for them when they begin to be needed.
 3. We have a plan for how we're going to use them in a
backwards-compatible way.

On 3, it sounds like we have a rough sketch of a plan but I'm still
unclear on some details.  In particular, we have an rsvd[8] at the end
but, if we're going to replace individual bits of it, we can't ever
shorten or re-name that array.  We could, potentially, do

union {
__u32 rsvd[8];
struct {
__u32 new_field;
};
};

and trust C to put all the fields of our anonymous struct at the top.
Otherwise, we've got to fill out our struct with more rsvd and that
gets to be a mess.

Another option would be to have

__u32 rsvd1;
__u32 rsvd2;
__u32 rsvd3;
/* etc... */

and we can replace them one at a time.

Again, my big point is that I want us to have a PLAN and not end up in
a scenario where we end up with rsvd[5] having magical meaning.  What
I see in DII does not give me confidence.  However, I do believe that
such a plan can exist.

--Jason

> I think the anonymous union with exactly same sized field is ok. We
> also try hard to be source compatible, but we have screwed up in the
> past and shrugged it off. The one example that comes to mind is
> extended structures at the bottom with new field, which the kernel
> automatically zero-extends for old userspace. But when you recompile,
> your new-old userspace might no longer clear the new fields because
> the ioctl code didn't start out by memset()ing the entire struct.

Also, we need to ensure that we memset everything to 0. :-)

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

Re: [Mesa-dev] [PATCH 1/9] drm/doc/rfc: i915 DG1 uAPI

2021-04-26 Thread Matthew Auld 

On 26/04/2021 16:11, Jason Ekstrand wrote:

On Mon, Apr 26, 2021 at 4:42 AM Matthew Auld  wrote:


Add an entry for the new uAPI needed for DG1. Also add the overall
upstream plan, including some notes for the TTM conversion.

v2(Daniel):
   - include the overall upstreaming plan
   - add a note for mmap, there are differences here for TTM vs i915
   - bunch of other suggestions from Daniel
v3:
  (Daniel)
   - add a note for set/get caching stuff
   - add some more docs for existing query and extensions stuff
   - add an actual code example for regions query
   - bunch of other stuff
  (Jason)
   - uAPI change(!):
 - try a simpler design with the placements extension
 - rather than have a generic setparam which can cover multiple
   use cases, have each extension be responsible for one thing
   only
v4:
  (Daniel)
   - add some more notes for ttm conversion
   - bunch of other stuff
  (Jason)
   - uAPI change(!):
 - drop all the extra rsvd members for the region_query and
   region_info, just keep the bare minimum needed for padding

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Thomas Hellström 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
Acked-by: Daniel Vetter 
Acked-by: Dave Airlie 
---
  Documentation/gpu/rfc/i915_gem_lmem.h   | 212 
  Documentation/gpu/rfc/i915_gem_lmem.rst | 130 +++
  Documentation/gpu/rfc/index.rst |   4 +
  3 files changed, 346 insertions(+)
  create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
  create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst

diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h 
b/Documentation/gpu/rfc/i915_gem_lmem.h
new file mode 100644
index ..7ed59b6202d5
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_gem_lmem.h
@@ -0,0 +1,212 @@
+/**
+ * enum drm_i915_gem_memory_class - Supported memory classes
+ */
+enum drm_i915_gem_memory_class {
+   /** @I915_MEMORY_CLASS_SYSTEM: System memory */
+   I915_MEMORY_CLASS_SYSTEM = 0,
+   /** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
+   I915_MEMORY_CLASS_DEVICE,
+};
+
+/**
+ * struct drm_i915_gem_memory_class_instance - Identify particular memory 
region
+ */
+struct drm_i915_gem_memory_class_instance {
+   /** @memory_class: See enum drm_i915_gem_memory_class */
+   __u16 memory_class;
+
+   /** @memory_instance: Which instance */
+   __u16 memory_instance;
+};
+
+/**
+ * struct drm_i915_memory_region_info - Describes one region as known to the
+ * driver.
+ *
+ * Note that we reserve some stuff here for potential future work. As an 
example
+ * we might want expose the capabilities(see @caps) for a given region, which
+ * could include things like if the region is CPU mappable/accessible, what are
+ * the supported mapping types etc.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS
+ * at &drm_i915_query_item.query_id.
+ */
+struct drm_i915_memory_region_info {
+   /** @region: The class:instance pair encoding */
+   struct drm_i915_gem_memory_class_instance region;
+
+   /** @pad: MBZ */
+   __u32 pad;
+
+   /** @caps: MBZ */
+   __u64 caps;
+
+   /** @probed_size: Memory probed by the driver (-1 = unknown) */
+   __u64 probed_size;
+
+   /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
+   __u64 unallocated_size;
+};
+
+/**
+ * struct drm_i915_query_memory_regions
+ *
+ * The region info query enumerates all regions known to the driver by filling
+ * in an array of struct drm_i915_memory_region_info structures.
+ *
+ * Example for getting the list of supported regions:
+ *
+ * .. code-block:: C
+ *
+ * struct drm_i915_query_memory_regions *info;
+ * struct drm_i915_query_item item = {
+ * .query_id = DRM_I915_QUERY_MEMORY_REGIONS;
+ * };
+ * struct drm_i915_query query = {
+ * .num_items = 1,
+ * .items_ptr = (uintptr_t)&item,
+ * };
+ * int err, i;
+ *
+ * // First query the size of the blob we need, this needs to be large
+ * // enough to hold our array of regions. The kernel will fill out the
+ * // item.length for us, which is the number of bytes we need.
+ * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
+ * if (err) ...
+ *
+ * info = calloc(1, item.length);
+ * // Now that we allocated the required number of bytes, we call the ioctl
+ * // again, this time with the data_ptr pointing to our newly allocated
+ * // blob, which the kernel can then populate with the all the region 
info.
+ * item.data_ptr = (uintptr_t)&info,
+ *
+ * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
+

Re: [Mesa-dev] [PATCH 1/9] drm/doc/rfc: i915 DG1 uAPI

2021-04-26 Thread Jason Ekstrand 
On Mon, Apr 26, 2021 at 10:31 AM Matthew Auld  wrote:
>
> On 26/04/2021 16:11, Jason Ekstrand wrote:
> > On Mon, Apr 26, 2021 at 4:42 AM Matthew Auld  wrote:
> >>
> >> Add an entry for the new uAPI needed for DG1. Also add the overall
> >> upstream plan, including some notes for the TTM conversion.
> >>
> >> v2(Daniel):
> >>- include the overall upstreaming plan
> >>- add a note for mmap, there are differences here for TTM vs i915
> >>- bunch of other suggestions from Daniel
> >> v3:
> >>   (Daniel)
> >>- add a note for set/get caching stuff
> >>- add some more docs for existing query and extensions stuff
> >>- add an actual code example for regions query
> >>- bunch of other stuff
> >>   (Jason)
> >>- uAPI change(!):
> >>  - try a simpler design with the placements extension
> >>  - rather than have a generic setparam which can cover multiple
> >>use cases, have each extension be responsible for one thing
> >>only
> >> v4:
> >>   (Daniel)
> >>- add some more notes for ttm conversion
> >>- bunch of other stuff
> >>   (Jason)
> >>- uAPI change(!):
> >>  - drop all the extra rsvd members for the region_query and
> >>region_info, just keep the bare minimum needed for padding
> >>
> >> Signed-off-by: Matthew Auld 
> >> Cc: Joonas Lahtinen 
> >> Cc: Thomas Hellström 
> >> Cc: Daniele Ceraolo Spurio 
> >> Cc: Lionel Landwerlin 
> >> Cc: Jon Bloomfield 
> >> Cc: Jordan Justen 
> >> Cc: Daniel Vetter 
> >> Cc: Kenneth Graunke 
> >> Cc: Jason Ekstrand 
> >> Cc: Dave Airlie 
> >> Cc: dri-de...@lists.freedesktop.org
> >> Cc: mesa-dev@lists.freedesktop.org
> >> Acked-by: Daniel Vetter 
> >> Acked-by: Dave Airlie 
> >> ---
> >>   Documentation/gpu/rfc/i915_gem_lmem.h   | 212 
> >>   Documentation/gpu/rfc/i915_gem_lmem.rst | 130 +++
> >>   Documentation/gpu/rfc/index.rst |   4 +
> >>   3 files changed, 346 insertions(+)
> >>   create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
> >>   create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst
> >>
> >> diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h 
> >> b/Documentation/gpu/rfc/i915_gem_lmem.h
> >> new file mode 100644
> >> index ..7ed59b6202d5
> >> --- /dev/null
> >> +++ b/Documentation/gpu/rfc/i915_gem_lmem.h
> >> @@ -0,0 +1,212 @@
> >> +/**
> >> + * enum drm_i915_gem_memory_class - Supported memory classes
> >> + */
> >> +enum drm_i915_gem_memory_class {
> >> +   /** @I915_MEMORY_CLASS_SYSTEM: System memory */
> >> +   I915_MEMORY_CLASS_SYSTEM = 0,
> >> +   /** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
> >> +   I915_MEMORY_CLASS_DEVICE,
> >> +};
> >> +
> >> +/**
> >> + * struct drm_i915_gem_memory_class_instance - Identify particular memory 
> >> region
> >> + */
> >> +struct drm_i915_gem_memory_class_instance {
> >> +   /** @memory_class: See enum drm_i915_gem_memory_class */
> >> +   __u16 memory_class;
> >> +
> >> +   /** @memory_instance: Which instance */
> >> +   __u16 memory_instance;
> >> +};
> >> +
> >> +/**
> >> + * struct drm_i915_memory_region_info - Describes one region as known to 
> >> the
> >> + * driver.
> >> + *
> >> + * Note that we reserve some stuff here for potential future work. As an 
> >> example
> >> + * we might want expose the capabilities(see @caps) for a given region, 
> >> which
> >> + * could include things like if the region is CPU mappable/accessible, 
> >> what are
> >> + * the supported mapping types etc.
> >> + *
> >> + * Note this is using both struct drm_i915_query_item and struct 
> >> drm_i915_query.
> >> + * For this new query we are adding the new query id 
> >> DRM_I915_QUERY_MEMORY_REGIONS
> >> + * at &drm_i915_query_item.query_id.
> >> + */
> >> +struct drm_i915_memory_region_info {
> >> +   /** @region: The class:instance pair encoding */
> >> +   struct drm_i915_gem_memory_class_instance region;
> >> +
> >> +   /** @pad: MBZ */
> >> +   __u32 pad;
> >> +
> >> +   /** @caps: MBZ */
> >> +   __u64 caps;
> >> +
> >> +   /** @probed_size: Memory probed by the driver (-1 = unknown) */
> >> +   __u64 probed_size;
> >> +
> >> +   /** @unallocated_size: Estimate of memory remaining (-1 = unknown) 
> >> */
> >> +   __u64 unallocated_size;
> >> +};
> >> +
> >> +/**
> >> + * struct drm_i915_query_memory_regions
> >> + *
> >> + * The region info query enumerates all regions known to the driver by 
> >> filling
> >> + * in an array of struct drm_i915_memory_region_info structures.
> >> + *
> >> + * Example for getting the list of supported regions:
> >> + *
> >> + * .. code-block:: C
> >> + *
> >> + * struct drm_i915_query_memory_regions *info;
> >> + * struct drm_i915_query_item item = {
> >> + * .query_id = DRM_I915_QUERY_MEMORY_REGIONS;
> >> + * };
> >> + * struct drm_i915_query query = {
> >> + * .num_items = 1,
> >> + * .items_ptr =

Re: [Mesa-dev] [PATCH 1/9] drm/doc/rfc: i915 DG1 uAPI

2021-04-26 Thread Daniel Vetter 
On Mon, Apr 26, 2021 at 11:25:09AM -0500, Jason Ekstrand wrote:
> On Mon, Apr 26, 2021 at 10:31 AM Matthew Auld  wrote:
> >
> > On 26/04/2021 16:11, Jason Ekstrand wrote:
> > > On Mon, Apr 26, 2021 at 4:42 AM Matthew Auld  
> > > wrote:
> > >>
> > >> Add an entry for the new uAPI needed for DG1. Also add the overall
> > >> upstream plan, including some notes for the TTM conversion.
> > >>
> > >> v2(Daniel):
> > >>- include the overall upstreaming plan
> > >>- add a note for mmap, there are differences here for TTM vs i915
> > >>- bunch of other suggestions from Daniel
> > >> v3:
> > >>   (Daniel)
> > >>- add a note for set/get caching stuff
> > >>- add some more docs for existing query and extensions stuff
> > >>- add an actual code example for regions query
> > >>- bunch of other stuff
> > >>   (Jason)
> > >>- uAPI change(!):
> > >>  - try a simpler design with the placements extension
> > >>  - rather than have a generic setparam which can cover multiple
> > >>use cases, have each extension be responsible for one thing
> > >>only
> > >> v4:
> > >>   (Daniel)
> > >>- add some more notes for ttm conversion
> > >>- bunch of other stuff
> > >>   (Jason)
> > >>- uAPI change(!):
> > >>  - drop all the extra rsvd members for the region_query and
> > >>region_info, just keep the bare minimum needed for padding
> > >>
> > >> Signed-off-by: Matthew Auld 
> > >> Cc: Joonas Lahtinen 
> > >> Cc: Thomas Hellström 
> > >> Cc: Daniele Ceraolo Spurio 
> > >> Cc: Lionel Landwerlin 
> > >> Cc: Jon Bloomfield 
> > >> Cc: Jordan Justen 
> > >> Cc: Daniel Vetter 
> > >> Cc: Kenneth Graunke 
> > >> Cc: Jason Ekstrand 
> > >> Cc: Dave Airlie 
> > >> Cc: dri-de...@lists.freedesktop.org
> > >> Cc: mesa-dev@lists.freedesktop.org
> > >> Acked-by: Daniel Vetter 
> > >> Acked-by: Dave Airlie 
> > >> ---
> > >>   Documentation/gpu/rfc/i915_gem_lmem.h   | 212 
> > >>   Documentation/gpu/rfc/i915_gem_lmem.rst | 130 +++
> > >>   Documentation/gpu/rfc/index.rst |   4 +
> > >>   3 files changed, 346 insertions(+)
> > >>   create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
> > >>   create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst
> > >>
> > >> diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h 
> > >> b/Documentation/gpu/rfc/i915_gem_lmem.h
> > >> new file mode 100644
> > >> index ..7ed59b6202d5
> > >> --- /dev/null
> > >> +++ b/Documentation/gpu/rfc/i915_gem_lmem.h
> > >> @@ -0,0 +1,212 @@
> > >> +/**
> > >> + * enum drm_i915_gem_memory_class - Supported memory classes
> > >> + */
> > >> +enum drm_i915_gem_memory_class {
> > >> +   /** @I915_MEMORY_CLASS_SYSTEM: System memory */
> > >> +   I915_MEMORY_CLASS_SYSTEM = 0,
> > >> +   /** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
> > >> +   I915_MEMORY_CLASS_DEVICE,
> > >> +};
> > >> +
> > >> +/**
> > >> + * struct drm_i915_gem_memory_class_instance - Identify particular 
> > >> memory region
> > >> + */
> > >> +struct drm_i915_gem_memory_class_instance {
> > >> +   /** @memory_class: See enum drm_i915_gem_memory_class */
> > >> +   __u16 memory_class;
> > >> +
> > >> +   /** @memory_instance: Which instance */
> > >> +   __u16 memory_instance;
> > >> +};
> > >> +
> > >> +/**
> > >> + * struct drm_i915_memory_region_info - Describes one region as known 
> > >> to the
> > >> + * driver.
> > >> + *
> > >> + * Note that we reserve some stuff here for potential future work. As 
> > >> an example
> > >> + * we might want expose the capabilities(see @caps) for a given region, 
> > >> which
> > >> + * could include things like if the region is CPU mappable/accessible, 
> > >> what are
> > >> + * the supported mapping types etc.
> > >> + *
> > >> + * Note this is using both struct drm_i915_query_item and struct 
> > >> drm_i915_query.
> > >> + * For this new query we are adding the new query id 
> > >> DRM_I915_QUERY_MEMORY_REGIONS
> > >> + * at &drm_i915_query_item.query_id.
> > >> + */
> > >> +struct drm_i915_memory_region_info {
> > >> +   /** @region: The class:instance pair encoding */
> > >> +   struct drm_i915_gem_memory_class_instance region;
> > >> +
> > >> +   /** @pad: MBZ */
> > >> +   __u32 pad;
> > >> +
> > >> +   /** @caps: MBZ */
> > >> +   __u64 caps;
> > >> +
> > >> +   /** @probed_size: Memory probed by the driver (-1 = unknown) */
> > >> +   __u64 probed_size;
> > >> +
> > >> +   /** @unallocated_size: Estimate of memory remaining (-1 = 
> > >> unknown) */
> > >> +   __u64 unallocated_size;
> > >> +};
> > >> +
> > >> +/**
> > >> + * struct drm_i915_query_memory_regions
> > >> + *
> > >> + * The region info query enumerates all regions known to the driver by 
> > >> filling
> > >> + * in an array of struct drm_i915_memory_region_info structures.
> > >> + *
> > >> + * Example for getting the list of supported regions:
> > >> + *
> >

Re: [Mesa-dev] [RFC] Linux Graphics Next: Explicit fences everywhere and no BO fences - initial proposal

2021-04-26 Thread Marek Olšák 
Thanks everybody. The initial proposal is dead. Here are some thoughts on
how to do it differently.

I think we can have direct command submission from userspace via
memory-mapped queues ("user queues") without changing window systems.

The memory management doesn't have to use GPU page faults like HMM.
Instead, it can wait for user queues of a specific process to go idle and
then unmap the queues, so that userspace can't submit anything. Buffer
evictions, pinning, etc. can be executed when all queues are unmapped
(suspended). Thus, no BO fences and page faults are needed.

Inter-process synchronization can use timeline semaphores. Userspace will
query the wait and signal value for a shared buffer from the kernel. The
kernel will keep a history of those queries to know which process is
responsible for signalling which buffer. There is only the wait-timeout
issue and how to identify the culprit. One of the solutions is to have the
GPU send all GPU signal commands and all timed out wait commands via an
interrupt to the kernel driver to monitor and validate userspace behavior.
With that, it can be identified whether the culprit is the waiting process
or the signalling process and which one. Invalid signal/wait parameters can
also be detected. The kernel can force-signal only the semaphores that time
out, and punish the processes which caused the timeout or used invalid
signal/wait parameters.

The question is whether this synchronization solution is robust enough for
dma_fence and whatever the kernel and window systems need.

Marek

On Tue, Apr 20, 2021 at 4:34 PM Daniel Stone  wrote:

> Hi,
>
> On Tue, 20 Apr 2021 at 20:30, Daniel Vetter  wrote:
>
>> The thing is, you can't do this in drm/scheduler. At least not without
>> splitting up the dma_fence in the kernel into separate memory fences
>> and sync fences
>
>
> I'm starting to think this thread needs its own glossary ...
>
> I propose we use 'residency fence' for execution fences which enact
> memory-residency operations, e.g. faulting in a page ultimately depending
> on GPU work retiring.
>
> And 'value fence' for the pure-userspace model suggested by timeline
> semaphores, i.e. fences being (*addr == val) rather than being able to look
> at ctx seqno.
>
> Cheers,
> Daniel
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] Trying to build a opencl dev env

2021-04-26 Thread Luke A. Guest 

Hi,

So, I've built a generic LLVM tree for mesa and other projects and 
installed it alongside SPIRV-LLVM-Translator into 
$HOME/opt/llvm-dev-reldebug.


I have installed libclc there too, although it wouldn't build as part of 
llvm and make install wouldn't install to the same llvm dir for some reason.


Now onto mesa, I cannot get this thing to build. It fails right at the 
start with the following error:


ninja: Entering directory `../builds/mesa-reldebug/'
ninja: error: '/usr/share/clc/spirv-mesa3d-.spv', needed by 
'src/compiler/nir/spirv-mesa3d-.spv.zstd', missing and no known rule to 
make it


It's looking int he wrong place for the clc files, which are in the 
above llvm dir. How do you specify where they are with this meson thing?


Thanks,
Luke.

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