* [PATCH 2/6] drm/i915: Use common LRU inactive vma bumping for unpin_from_display
2017-01-19 19:26 [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Chris Wilson
@ 2017-01-19 19:26 ` Chris Wilson
2017-01-20 12:32 ` Joonas Lahtinen
2017-01-19 19:26 ` [PATCH 3/6] drm/i915: Reject vma creation larger than address space Chris Wilson
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2017-01-19 19:26 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/6] drm/i915: Use common LRU inactive vma bumping for unpin_from_display
2017-01-19 19:26 ` [PATCH 2/6] drm/i915: Use common LRU inactive vma bumping for unpin_from_display Chris Wilson
@ 2017-01-20 12:32 ` Joonas Lahtinen
0 siblings, 0 replies; 17+ messages in thread
From: Joonas Lahtinen @ 2017-01-20 12:32 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/6] drm/i915: Reject vma creation larger than address space
2017-01-19 19:26 [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Chris Wilson
2017-01-19 19:26 ` [PATCH 2/6] drm/i915: Use common LRU inactive vma bumping for unpin_from_display Chris Wilson
@ 2017-01-19 19:26 ` Chris Wilson
2017-01-20 12:06 ` Joonas Lahtinen
2017-01-19 19:26 ` [PATCH 4/6] drm/i915: Treat an error from i915_vma_instance() as unlikely Chris Wilson
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2017-01-19 19:26 UTC (permalink / raw)
To: intel-gfx; +Cc: Mika Kuoppala
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 4/6] drm/i915: Treat an error from i915_vma_instance() as unlikely
2017-01-19 19:26 [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Chris Wilson
2017-01-19 19:26 ` [PATCH 2/6] drm/i915: Use common LRU inactive vma bumping for unpin_from_display Chris Wilson
2017-01-19 19:26 ` [PATCH 3/6] drm/i915: Reject vma creation larger than address space Chris Wilson
@ 2017-01-19 19:26 ` Chris Wilson
2017-01-20 12:34 ` Joonas Lahtinen
2017-01-19 19:26 ` [PATCH 5/6] drm/i915: Assert the drm_mm_node is allocated when on the VM lists Chris Wilson
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2017-01-19 19:26 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/6] drm/i915: Treat an error from i915_vma_instance() as unlikely
2017-01-19 19:26 ` [PATCH 4/6] drm/i915: Treat an error from i915_vma_instance() as unlikely Chris Wilson
@ 2017-01-20 12:34 ` Joonas Lahtinen
0 siblings, 0 replies; 17+ messages in thread
From: Joonas Lahtinen @ 2017-01-20 12:34 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/6] drm/i915: Assert the drm_mm_node is allocated when on the VM lists
2017-01-19 19:26 [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Chris Wilson
` (2 preceding siblings ...)
2017-01-19 19:26 ` [PATCH 4/6] drm/i915: Treat an error from i915_vma_instance() as unlikely Chris Wilson
@ 2017-01-19 19:26 ` Chris Wilson
2017-01-20 12:36 ` Joonas Lahtinen
2017-01-19 19:26 ` [PATCH 6/6] drm/i915: Assert that created vma has a whole number of pages Chris Wilson
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2017-01-19 19:26 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 6/6] drm/i915: Assert that created vma has a whole number of pages
2017-01-19 19:26 [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Chris Wilson
` (3 preceding siblings ...)
2017-01-19 19:26 ` [PATCH 5/6] drm/i915: Assert the drm_mm_node is allocated when on the VM lists Chris Wilson
@ 2017-01-19 19:26 ` Chris Wilson
2017-01-20 12:38 ` Joonas Lahtinen
2017-01-19 19:44 ` [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Ville Syrjälä
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2017-01-19 19:26 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2
2017-01-19 19:26 [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Chris Wilson
` (4 preceding siblings ...)
2017-01-19 19:26 ` [PATCH 6/6] drm/i915: Assert that created vma has a whole number of pages Chris Wilson
@ 2017-01-19 19:44 ` Ville Syrjälä
2017-01-20 10:36 ` Chris Wilson
2017-01-19 19:54 ` ✗ Fi.CI.BAT: failure for series starting with [1/6] " Patchwork
2017-01-20 12:29 ` [PATCH 1/6] " Joonas Lahtinen
7 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2017-01-19 19:44 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2
2017-01-19 19:44 ` [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Ville Syrjälä
@ 2017-01-20 10:36 ` Chris Wilson
2017-01-20 12:37 ` Joonas Lahtinen
0 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2017-01-20 10:36 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2
2017-01-20 10:36 ` Chris Wilson
@ 2017-01-20 12:37 ` Joonas Lahtinen
0 siblings, 0 replies; 17+ messages in thread
From: Joonas Lahtinen @ 2017-01-20 12:37 UTC (permalink / raw)
To: Chris Wilson, Ville Syrjälä; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/6] drm/i915: Pass around gen to tile_height to special case gen2
2017-01-19 19:26 [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Chris Wilson
` (5 preceding siblings ...)
2017-01-19 19:44 ` [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Ville Syrjälä
@ 2017-01-19 19:54 ` Patchwork
2017-01-20 12:29 ` [PATCH 1/6] " Joonas Lahtinen
7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2017-01-19 19:54 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2
2017-01-19 19:26 [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2 Chris Wilson
` (6 preceding siblings ...)
2017-01-19 19:54 ` ✗ Fi.CI.BAT: failure for series starting with [1/6] " Patchwork
@ 2017-01-20 12:29 ` Joonas Lahtinen
2017-01-21 10:30 ` Chris Wilson
7 siblings, 1 reply; 17+ messages in thread
From: Joonas Lahtinen @ 2017-01-20 12:29 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] drm/i915: Pass around gen to tile_height to special case gen2
2017-01-20 12:29 ` [PATCH 1/6] " Joonas Lahtinen
@ 2017-01-21 10:30 ` Chris Wilson
0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2017-01-21 10:30 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 17+ messages in thread