Re: [Intel-gfx] [PATCH 04/15] drm/i915: introduce gem object page_sizes

[Chris Wilson]

On Wed, May 31, 2017 at 07:51:59PM +0100, Matthew Auld wrote:
> + max_page_size = BIT(fls64(obj->mm.page_sizes.sg)-1);
> +
> + /* If were are actually dealing with a single page-size, mark it so */
> + if (IS_ALIGNED(obj->mm.page_sizes.phys, max_page_size))
> + obj->mm.page_sizes.sg = max_page_size;

Pardon?

If the physical page sizes is a multiple of the max sg->length (rounded
to GTT page sizes), only use the max?

page_sizes.sg will already express the smallest unit that can be used to
for the whole object (as well as the largest one that may be used
opportunistically).

The current kselftests that mix sg chunk sizes are ignorant of the large
pages sizes, it would be useful to add some that did mix huge pages.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/15] drm/i915: introduce gem object page_sizes

[Chris Wilson]

On Wed, May 31, 2017 at 07:51:59PM +0100, Matthew Auld wrote:
>   err = mutex_lock_interruptible(>mm.lock);
> @@ -2533,7 +2543,33 @@ int __i915_gem_object_get_pages(struct 
> drm_i915_gem_object *obj)
>  
>  unlock:
>   mutex_unlock(>mm.lock);
> - return err;
> +
> + if (err)
> + return err;
> +
> + for_each_sg(obj->mm.pages->sgl, sg, obj->mm.pages->nents, i)
> + sg_mask |= sg->length;

This is the worst place to put a loop over sg. Not only is this
synchronous, but pages may not have been allocated yet. Using
set_pages was at least correct in that regard! The interface I will keep
nagging for is for each task to compute sg_mask as they populate the
scatterlist, and then they pass sg_mask to
__i915_gem_object_set_pages() alongside the pages.

Since this didn't fail BAT, that only means we do not have any userptr
tests there...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 04/15] drm/i915: introduce gem object page_sizes

[Matthew Auld]

We need to track the possible page sizes given the layout of the sg
table, in preparation for supporting huge gtt pages. Note that this does
in any way represent the real gtt page size usage.

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Chris Wilson 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_drv.h|  2 ++
 drivers/gpu/drm/i915/i915_gem.c| 38 +-
 drivers/gpu/drm/i915/i915_gem_object.h |  5 +
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8be48e5d8c1f..e6d1e1df4454 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2896,6 +2896,8 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define USES_PPGTT(dev_priv)   (i915.enable_ppgtt)
 #define USES_FULL_PPGTT(dev_priv)  (i915.enable_ppgtt >= 2)
 #define USES_FULL_48BIT_PPGTT(dev_priv)(i915.enable_ppgtt == 3)
+#define HAS_PAGE_SIZE(dev_priv, page_size) \
+   ((dev_priv)->info.page_size_mask & (page_size))
 
 #define HAS_OVERLAY(dev_priv)   ((dev_priv)->info.has_overlay)
 #define OVERLAY_NEEDS_PHYSICAL(dev_priv) \
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 30f9af590969..e8ebf39448a5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2294,6 +2294,8 @@ void __i915_gem_object_put_pages(struct 
drm_i915_gem_object *obj,
if (!IS_ERR(pages))
obj->ops->put_pages(obj, pages);
 
+   obj->mm.page_sizes.phys = obj->mm.page_sizes.sg = 0;
+
 unlock:
mutex_unlock(>mm.lock);
 }
@@ -2473,6 +2475,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object 
*obj)
 void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
 struct sg_table *pages)
 {
+
lockdep_assert_held(>mm.lock);
 
obj->mm.get_page.sg_pos = pages->sgl;
@@ -2516,6 +2519,13 @@ static int i915_gem_object_get_pages(struct 
drm_i915_gem_object *obj)
  */
 int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
 {
+   struct drm_i915_private *i915 = to_i915(obj->base.dev);
+   unsigned long supported_page_sizes = INTEL_INFO(i915)->page_size_mask;
+   struct scatterlist *sg;
+   unsigned int sg_mask = 0;
+   unsigned int i;
+   unsigned int bit;
+   unsigned int max_page_size;
int err;
 
err = mutex_lock_interruptible(>mm.lock);
@@ -2533,7 +2543,33 @@ int __i915_gem_object_get_pages(struct 
drm_i915_gem_object *obj)
 
 unlock:
mutex_unlock(>mm.lock);
-   return err;
+
+   if (err)
+   return err;
+
+   for_each_sg(obj->mm.pages->sgl, sg, obj->mm.pages->nents, i)
+   sg_mask |= sg->length;
+
+   GEM_BUG_ON(!sg_mask);
+
+   obj->mm.page_sizes.phys = sg_mask;
+
+   obj->mm.page_sizes.sg = 0;
+
+   for_each_set_bit(bit, _page_sizes, BITS_PER_LONG) {
+   if (obj->mm.page_sizes.phys & ~0u << bit)
+   obj->mm.page_sizes.sg |= BIT(bit);
+   }
+
+   max_page_size = BIT(fls64(obj->mm.page_sizes.sg)-1);
+
+   /* If were are actually dealing with a single page-size, mark it so */
+   if (IS_ALIGNED(obj->mm.page_sizes.phys, max_page_size))
+   obj->mm.page_sizes.sg = max_page_size;
+
+   GEM_BUG_ON(!HAS_PAGE_SIZE(i915, obj->mm.page_sizes.sg));
+
+   return 0;
 }
 
 /* The 'mapping' part of i915_gem_object_pin_map() below */
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h 
b/drivers/gpu/drm/i915/i915_gem_object.h
index 35e1a27729dc..6db34eac9794 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -129,6 +129,11 @@ struct drm_i915_gem_object {
struct sg_table *pages;
void *mapping;
 
+   struct i915_page_sizes {
+   unsigned int phys;
+   unsigned int sg;
+   } page_sizes;
+
struct i915_gem_object_page_iter {
struct scatterlist *sg_pos;
unsigned int sg_idx; /* in pages, but 32bit eek! */
-- 
2.9.4

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