[PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2

[PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2

[Chris Wilson]

gen2 has a different tiling pattern, and a fixed tile_height. Pass
along the gen so that we the computed tile_row is large enough to align
with gen2 fences.

Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c        |  3 ++-
 drivers/gpu/drm/i915/i915_gem_object.h | 12 ++++++------
 drivers/gpu/drm/i915/i915_gem_tiling.c |  2 +-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b2f8ac1386a2..9412eba5e0a8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1696,7 +1696,8 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 
 static unsigned int tile_row_pages(struct drm_i915_gem_object *obj)
 {
-	return i915_gem_object_get_tile_row_size(obj) >> PAGE_SHIFT;
+	int gen = INTEL_GEN(to_i915(obj->base.dev));
+	return i915_gem_object_get_tile_row_size(obj, gen) >> PAGE_SHIFT;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 290eaa7fc9eb..33a7d031e749 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -318,23 +318,23 @@ i915_gem_object_get_stride(struct drm_i915_gem_object *obj)
 }
 
 static inline unsigned int
-i915_gem_tile_height(unsigned int tiling)
+i915_gem_tile_height(int gen, unsigned int tiling)
 {
 	GEM_BUG_ON(!tiling);
-	return tiling == I915_TILING_Y ? 32 : 8;
+	return gen == 2 ? 16 : tiling == I915_TILING_Y ? 32 : 8;
 }
 
 static inline unsigned int
-i915_gem_object_get_tile_height(struct drm_i915_gem_object *obj)
+i915_gem_object_get_tile_height(struct drm_i915_gem_object *obj, int gen)
 {
-	return i915_gem_tile_height(i915_gem_object_get_tiling(obj));
+	return i915_gem_tile_height(gen, i915_gem_object_get_tiling(obj));
 }
 
 static inline unsigned int
-i915_gem_object_get_tile_row_size(struct drm_i915_gem_object *obj)
+i915_gem_object_get_tile_row_size(struct drm_i915_gem_object *obj, int gen)
 {
 	return (i915_gem_object_get_stride(obj) *
-		i915_gem_object_get_tile_height(obj));
+		i915_gem_object_get_tile_height(obj, gen));
 }
 
 int i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index b1361cfd4c5c..bfa4a563ebf1 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -81,7 +81,7 @@ u32 i915_gem_fence_size(struct drm_i915_private *i915,
 	GEM_BUG_ON(!stride);
 
 	if (INTEL_GEN(i915) >= 4) {
-		stride *= i915_gem_tile_height(tiling);
+		stride *= i915_gem_tile_height(INTEL_GEN(i915), tiling);
 		GEM_BUG_ON(!IS_ALIGNED(stride, I965_FENCE_PAGE));
 		return roundup(size, stride);
 	}
-- 
2.11.0

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

[PATCH 2/6] drm/i915: Use common LRU inactive vma bumping for unpin_from_display

[Chris Wilson]

Now that i915_gem_object_bump_inactive_ggtt() exists, also make use of
it for the LRU bumping from i915_gem_object_unpin_from_display()

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9412eba5e0a8..fdb6e0ece18c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3548,8 +3548,7 @@ i915_gem_object_unpin_from_display_plane(struct i915_vma *vma)
 		vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
 
 	/* Bump the LRU to try and avoid premature eviction whilst flipping  */
-	if (!i915_vma_is_active(vma))
-		list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
+	i915_gem_object_bump_inactive_ggtt(vma->obj);
 
 	i915_vma_unpin(vma);
 }
-- 
2.11.0

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

[PATCH 3/6] drm/i915: Reject vma creation larger than address space

[Chris Wilson]

Disallow creation of a vma that is larger than the available address
space, or triggers an overflow on fence expansion.

Testcase: igt/gem_exec_reloc/gtt-32
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_vma.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 635f2635b1f2..e58d8799bee2 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -77,7 +77,7 @@ vma_create(struct drm_i915_gem_object *obj,
 	struct rb_node *rb, **p;
 	int i;
 
-	vma = kmem_cache_zalloc(to_i915(obj->base.dev)->vmas, GFP_KERNEL);
+	vma = kmem_cache_zalloc(vm->i915->vmas, GFP_KERNEL);
 	if (vma == NULL)
 		return ERR_PTR(-ENOMEM);
 
@@ -85,7 +85,6 @@ vma_create(struct drm_i915_gem_object *obj,
 	for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
 		init_request_active(&vma->last_read[i], i915_vma_retire);
 	init_request_active(&vma->last_fence, NULL);
-	list_add(&vma->vm_link, &vm->unbound_list);
 	vma->vm = vm;
 	vma->obj = obj;
 	vma->size = obj->base.size;
@@ -107,11 +106,20 @@ vma_create(struct drm_i915_gem_object *obj,
 		}
 	}
 
+	if (unlikely(vma->size > vm->total))
+		goto err_vma;
+
 	if (i915_is_ggtt(vm)) {
-		GEM_BUG_ON(overflows_type(vma->size, u32));
+		if (unlikely(overflows_type(vma->size, u32)))
+			goto err_vma;
+
 		vma->fence_size = i915_gem_fence_size(vm->i915, vma->size,
 						      i915_gem_object_get_tiling(obj),
 						      i915_gem_object_get_stride(obj));
+		if (unlikely(vma->fence_size < vma->size || /* overflow */
+			     vma->fence_size > vm->total))
+			goto err_vma;
+
 		GEM_BUG_ON(!IS_ALIGNED(vma->fence_size, I915_GTT_MIN_ALIGNMENT));
 
 		vma->fence_alignment = i915_gem_fence_alignment(vm->i915, vma->size,
@@ -140,8 +148,13 @@ vma_create(struct drm_i915_gem_object *obj,
 	}
 	rb_link_node(&vma->obj_node, rb, p);
 	rb_insert_color(&vma->obj_node, &obj->vma_tree);
+	list_add(&vma->vm_link, &vm->unbound_list);
 
 	return vma;
+
+err_vma:
+	kmem_cache_free(vm->i915->vmas, vma);
+	return ERR_PTR(-E2BIG);
 }
 
 static struct i915_vma *
-- 
2.11.0

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

[PATCH 4/6] drm/i915: Treat an error from i915_vma_instance() as unlikely

[Chris Wilson]

When pinning into the global GTT, an error from creating the VMA is
unlikely, so mark it so.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fdb6e0ece18c..ae13450fcbe2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3680,7 +3680,7 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
 	lockdep_assert_held(&obj->base.dev->struct_mutex);
 
 	vma = i915_vma_instance(obj, vm, view);
-	if (IS_ERR(vma))
+	if (unlikely(IS_ERR(vma)))
 		return vma;
 
 	if (i915_vma_misplaced(vma, size, alignment, flags)) {
-- 
2.11.0

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

[PATCH 5/6] drm/i915: Assert the drm_mm_node is allocated when on the VM lists

[Chris Wilson]

Before moving the vma between the VM active/inactive lists, assert that
the node is still allocated.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 2 ++
 drivers/gpu/drm/i915/i915_vma.c        | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 61cc0fcae3d8..127d698e7c84 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -702,6 +702,8 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
 		goto err_pages;
 	}
 
+	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
+
 	vma->pages = obj->mm.pages;
 	vma->flags |= I915_VMA_GLOBAL_BIND;
 	__i915_vma_set_map_and_fenceable(vma);
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index e58d8799bee2..ecb495b1c5d3 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -45,6 +45,7 @@ i915_vma_retire(struct i915_gem_active *active,
 	if (i915_vma_is_active(vma))
 		return;
 
+	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
 	if (unlikely(i915_vma_is_closed(vma) && !i915_vma_is_pinned(vma)))
 		WARN_ON(i915_vma_unbind(vma));
@@ -493,6 +494,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 		GEM_BUG_ON(vma->node.start < start);
 		GEM_BUG_ON(vma->node.start + vma->node.size > end);
 	}
+	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
 
 	list_move_tail(&obj->global_link, &dev_priv->mm.bound_list);
-- 
2.11.0

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

[PATCH 6/6] drm/i915: Assert that created vma has a whole number of pages

[Chris Wilson]

VMA (and their objects) are supposed to composed of whole pages. Add an
assert to catch any invalid construct when we create the VMA.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_vma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index ecb495b1c5d3..559966dc3d35 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -110,6 +110,8 @@ vma_create(struct drm_i915_gem_object *obj,
 	if (unlikely(vma->size > vm->total))
 		goto err_vma;
 
+	GEM_BUG_ON(!IS_ALIGNED(vma->size, I915_GTT_PAGE_SIZE));
+
 	if (i915_is_ggtt(vm)) {
 		if (unlikely(overflows_type(vma->size, u32)))
 			goto err_vma;
-- 
2.11.0

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

Re: [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2

[Ville Syrjälä]

On Thu, Jan 19, 2017 at 07:26:54PM +0000, Chris Wilson wrote:
> gen2 has a different tiling pattern, and a fixed tile_height. Pass
> along the gen so that we the computed tile_row is large enough to align
> with gen2 fences.
> 
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c        |  3 ++-
>  drivers/gpu/drm/i915/i915_gem_object.h | 12 ++++++------
>  drivers/gpu/drm/i915/i915_gem_tiling.c |  2 +-
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index b2f8ac1386a2..9412eba5e0a8 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1696,7 +1696,8 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  
>  static unsigned int tile_row_pages(struct drm_i915_gem_object *obj)
>  {
> -	return i915_gem_object_get_tile_row_size(obj) >> PAGE_SHIFT;
> +	int gen = INTEL_GEN(to_i915(obj->base.dev));
> +	return i915_gem_object_get_tile_row_size(obj, gen) >> PAGE_SHIFT;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
> index 290eaa7fc9eb..33a7d031e749 100644
> --- a/drivers/gpu/drm/i915/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/i915_gem_object.h
> @@ -318,23 +318,23 @@ i915_gem_object_get_stride(struct drm_i915_gem_object *obj)
>  }
>  
>  static inline unsigned int
> -i915_gem_tile_height(unsigned int tiling)
> +i915_gem_tile_height(int gen, unsigned int tiling)
>  {
>  	GEM_BUG_ON(!tiling);
> -	return tiling == I915_TILING_Y ? 32 : 8;
> +	return gen == 2 ? 16 : tiling == I915_TILING_Y ? 32 : 8;

Still inaccurate for 915+Y, but if this is only used to compute the tile
row size it'll just give a needlessly large answer and so should be
safe.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  }
>  
>  static inline unsigned int
> -i915_gem_object_get_tile_height(struct drm_i915_gem_object *obj)
> +i915_gem_object_get_tile_height(struct drm_i915_gem_object *obj, int gen)
>  {
> -	return i915_gem_tile_height(i915_gem_object_get_tiling(obj));
> +	return i915_gem_tile_height(gen, i915_gem_object_get_tiling(obj));
>  }
>  
>  static inline unsigned int
> -i915_gem_object_get_tile_row_size(struct drm_i915_gem_object *obj)
> +i915_gem_object_get_tile_row_size(struct drm_i915_gem_object *obj, int gen)
>  {
>  	return (i915_gem_object_get_stride(obj) *
> -		i915_gem_object_get_tile_height(obj));
> +		i915_gem_object_get_tile_height(obj, gen));
>  }
>  
>  int i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index b1361cfd4c5c..bfa4a563ebf1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -81,7 +81,7 @@ u32 i915_gem_fence_size(struct drm_i915_private *i915,
>  	GEM_BUG_ON(!stride);
>  
>  	if (INTEL_GEN(i915) >= 4) {
> -		stride *= i915_gem_tile_height(tiling);
> +		stride *= i915_gem_tile_height(INTEL_GEN(i915), tiling);
>  		GEM_BUG_ON(!IS_ALIGNED(stride, I965_FENCE_PAGE));
>  		return roundup(size, stride);
>  	}
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/6] drm/i915: Pass around gen to tile_height to special case gen2

[Patchwork]

== Series Details ==

Series: series starting with [1/6] drm/i915: Pass around gen to tile_height to special case gen2
URL   : https://patchwork.freedesktop.org/series/18241/
State : failure

== Summary ==

Series 18241v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/18241/revisions/1/mbox/

Test gem_busy:
        Subgroup basic-busy-default:
                pass       -> FAIL       (fi-ivb-3520m)
        Subgroup basic-hang-default:
                fail       -> PASS       (fi-ivb-3520m)

fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:79   pass:66   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:224  dwarn:0   dfail:0   fail:1   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

6155f94d32e438a9656de680897060bd2245629a drm-tip: 2017y-01m-19d-18h-24m-49s UTC integration manifest
acdbbaf drm/i915: Assert that created vma has a whole number of pages
d04527b drm/i915: Assert the drm_mm_node is allocated when on the VM lists
f73d0c6 drm/i915: Treat an error from i915_vma_instance() as unlikely
f282a1a drm/i915: Reject vma creation larger than address space
12e960a drm/i915: Use common LRU inactive vma bumping for unpin_from_display
1d188ca drm/i915: Pass around gen to tile_height to special case gen2

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3550/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2

[Chris Wilson]

On Thu, Jan 19, 2017 at 09:44:23PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 19, 2017 at 07:26:54PM +0000, Chris Wilson wrote:
> > gen2 has a different tiling pattern, and a fixed tile_height. Pass
> > along the gen so that we the computed tile_row is large enough to align
> > with gen2 fences.
> > 
> > Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c        |  3 ++-
> >  drivers/gpu/drm/i915/i915_gem_object.h | 12 ++++++------
> >  drivers/gpu/drm/i915/i915_gem_tiling.c |  2 +-
> >  3 files changed, 9 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index b2f8ac1386a2..9412eba5e0a8 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1696,7 +1696,8 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
> >  
> >  static unsigned int tile_row_pages(struct drm_i915_gem_object *obj)
> >  {
> > -	return i915_gem_object_get_tile_row_size(obj) >> PAGE_SHIFT;
> > +	int gen = INTEL_GEN(to_i915(obj->base.dev));
> > +	return i915_gem_object_get_tile_row_size(obj, gen) >> PAGE_SHIFT;
> >  }
> >  
> >  /**
> > diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
> > index 290eaa7fc9eb..33a7d031e749 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_object.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_object.h
> > @@ -318,23 +318,23 @@ i915_gem_object_get_stride(struct drm_i915_gem_object *obj)
> >  }
> >  
> >  static inline unsigned int
> > -i915_gem_tile_height(unsigned int tiling)
> > +i915_gem_tile_height(int gen, unsigned int tiling)
> >  {
> >  	GEM_BUG_ON(!tiling);
> > -	return tiling == I915_TILING_Y ? 32 : 8;
> > +	return gen == 2 ? 16 : tiling == I915_TILING_Y ? 32 : 8;
> 
> Still inaccurate for 915+Y, but if this is only used to compute the tile
> row size it'll just give a needlessly large answer and so should be
> safe.

The alternative is that this code is only relevant for gen4+. I wonder
if renaming and restricting at the caller is more tidy.
-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: [PATCH 3/6] drm/i915: Reject vma creation larger than address space

[Joonas Lahtinen]

On to, 2017-01-19 at 19:26 +0000, Chris Wilson wrote:
> Disallow creation of a vma that is larger than the available address
> space, or triggers an overflow on fence expansion.
> 
> Testcase: igt/gem_exec_reloc/gtt-32
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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: [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2

[Joonas Lahtinen]

On to, 2017-01-19 at 19:26 +0000, Chris Wilson wrote:
> gen2 has a different tiling pattern, and a fixed tile_height. Pass
> along the gen so that we the computed tile_row is large enough to align
> with gen2 fences.
> 
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

>  static inline unsigned int
> -i915_gem_object_get_tile_height(struct drm_i915_gem_object *obj)
> +i915_gem_object_get_tile_height(struct drm_i915_gem_object *obj, int gen)
>  {
> -	return i915_gem_tile_height(i915_gem_object_get_tiling(obj));
> +	return i915_gem_tile_height(gen, i915_gem_object_get_tiling(obj));
>  }
>  
>  static inline unsigned int
> -i915_gem_object_get_tile_row_size(struct drm_i915_gem_object *obj)
> +i915_gem_object_get_tile_row_size(struct drm_i915_gem_object *obj, int gen)
>  {
>  	return (i915_gem_object_get_stride(obj) *
> -		i915_gem_object_get_tile_height(obj));
> +		i915_gem_object_get_tile_height(obj, gen));
>  }

Compiler not smart enough to optimize these two if you grab it from obj
with INTEL_GEN? Anyway;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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: [PATCH 2/6] drm/i915: Use common LRU inactive vma bumping for unpin_from_display

[Joonas Lahtinen]

On to, 2017-01-19 at 19:26 +0000, Chris Wilson wrote:
> Now that i915_gem_object_bump_inactive_ggtt() exists, also make use of
> it for the LRU bumping from i915_gem_object_unpin_from_display()
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

>  	/* Bump the LRU to try and avoid premature eviction whilst flipping  */
> -	if (!i915_vma_is_active(vma))
> -		list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
> +	i915_gem_object_bump_inactive_ggtt(vma->obj);

Infrequently called function, so the loop within doesn't hurt;

Reviewed-by: Jonas Lahtinen <joonas.lahtinen@linux.intel.com>

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: [PATCH 4/6] drm/i915: Treat an error from i915_vma_instance() as unlikely

[Joonas Lahtinen]

On to, 2017-01-19 at 19:26 +0000, Chris Wilson wrote:
> When pinning into the global GTT, an error from creating the VMA is
> unlikely, so mark it so.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>
 
>  	vma = i915_vma_instance(obj, vm, view);
> -	if (IS_ERR(vma))
> +	if (unlikely(IS_ERR(vma)))
>  		return vma;

I bet we have many spots similar to this, any ideas how we could get
some good code coverage testing data from the selftests? I think we
only now optimize when it appears during debugging some specific
problem.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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: [PATCH 5/6] drm/i915: Assert the drm_mm_node is allocated when on the VM lists

[Joonas Lahtinen]

On to, 2017-01-19 at 19:26 +0000, Chris Wilson wrote:
> Before moving the vma between the VM active/inactive lists, assert that
> the node is still allocated.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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: [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2

[Joonas Lahtinen]

On pe, 2017-01-20 at 10:36 +0000, Chris Wilson wrote:
> On Thu, Jan 19, 2017 at 09:44:23PM +0200, Ville Syrjälä wrote:
> > 
> > Still inaccurate for 915+Y, but if this is only used to compute the tile
> > row size it'll just give a needlessly large answer and so should be
> > safe.
> 
> The alternative is that this code is only relevant for gen4+. I wonder
> if renaming and restricting at the caller is more tidy.

I think caller restriction would be better, addresses my concern about
the gen parameter too.

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: [PATCH 6/6] drm/i915: Assert that created vma has a whole number of pages

[Joonas Lahtinen]

On to, 2017-01-19 at 19:26 +0000, Chris Wilson wrote:
> VMA (and their objects) are supposed to composed of whole pages. Add an
> assert to catch any invalid construct when we create the VMA.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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: [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2

[Chris Wilson]

On Fri, Jan 20, 2017 at 02:29:38PM +0200, Joonas Lahtinen wrote:
> On to, 2017-01-19 at 19:26 +0000, Chris Wilson wrote:
> > gen2 has a different tiling pattern, and a fixed tile_height. Pass
> > along the gen so that we the computed tile_row is large enough to align
> > with gen2 fences.
> > 
> > Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> <SNIP>
> 
> >  static inline unsigned int
> > -i915_gem_object_get_tile_height(struct drm_i915_gem_object *obj)
> > +i915_gem_object_get_tile_height(struct drm_i915_gem_object *obj, int gen)
> >  {
> > -	return i915_gem_tile_height(i915_gem_object_get_tiling(obj));
> > +	return i915_gem_tile_height(gen, i915_gem_object_get_tiling(obj));
> >  }
> >  
> >  static inline unsigned int
> > -i915_gem_object_get_tile_row_size(struct drm_i915_gem_object *obj)
> > +i915_gem_object_get_tile_row_size(struct drm_i915_gem_object *obj, int gen)
> >  {
> >  	return (i915_gem_object_get_stride(obj) *
> > -		i915_gem_object_get_tile_height(obj));
> > +		i915_gem_object_get_tile_height(obj, gen));
> >  }
> 
> Compiler not smart enough to optimize these two if you grab it from obj
> with INTEL_GEN? Anyway;

It's the curse of the headers. :(
-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