On Wed, Feb 01, 2017 at 02:33:22PM +0000, Tvrtko Ursulin wrote:
> 
> On 19/01/2017 11:41, Chris Wilson wrote:
> >Exercise creating rotated VMA and checking the page order within.
> >
> >v2: Be more creative in rotated params
> >
> >Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> >---
> > drivers/gpu/drm/i915/selftests/i915_vma.c | 177 
> > ++++++++++++++++++++++++++++++
> > 1 file changed, 177 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c 
> >b/drivers/gpu/drm/i915/selftests/i915_vma.c
> >index b45b392444e4..2bda93f53b47 100644
> >--- a/drivers/gpu/drm/i915/selftests/i915_vma.c
> >+++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
> >@@ -310,11 +310,188 @@ static int igt_vma_pin1(void *arg)
> >     return err;
> > }
> >
> >+static unsigned long rotated_index(const struct intel_rotation_info *r,
> >+                               unsigned int n,
> >+                               unsigned int x,
> >+                               unsigned int y)
> >+{
> >+    return (r->plane[n].stride * (r->plane[n].height - y - 1) +
> >+            r->plane[n].offset + x);
> >+}
> >+
> >+static struct scatterlist *
> >+assert_rotated(struct drm_i915_gem_object *obj,
> >+           const struct intel_rotation_info *r, unsigned int n,
> >+           struct scatterlist *sg)
> >+{
> >+    unsigned int x, y;
> >+
> >+    for (x = 0; x < r->plane[n].width; x++) {
> >+            for (y = 0; y < r->plane[n].height; y++) {
> >+                    unsigned long src_idx;
> >+                    dma_addr_t src;
> >+
> >+                    src_idx = rotated_index(r, n, x, y);
> >+                    src = i915_gem_object_get_dma_address(obj, src_idx);
> >+
> >+                    if (sg_dma_len(sg) != PAGE_SIZE) {
> >+                            pr_err("Invalid sg.length, found %d, expected 
> >%lu for rotated page (%d, %d) [src index %lu]\n",
> >+                                   sg_dma_len(sg), PAGE_SIZE,
> >+                                   x, y, src_idx);
> >+                            return ERR_PTR(-EINVAL);
> >+                    }
> >+
> >+                    if (sg_dma_address(sg) != src) {
> >+                            pr_err("Invalid address for rotated page (%d, 
> >%d) [src index %lu]\n",
> >+                                   x, y, src_idx);
> >+                            return ERR_PTR(-EINVAL);
> >+                    }
> >+
> >+                    sg = ____sg_next(sg);
> >+            }
> >+    }
> >+
> >+    return sg;
> >+}
> >+
> >+static unsigned int rotated_size(const struct intel_rotation_plane_info *a,
> >+                             const struct intel_rotation_plane_info *b)
> >+{
> >+    return a->width * a->height + b->width * b->height;
> >+}
> >+
> >+static int igt_vma_rotate(void *arg)
> >+{
> >+    struct drm_i915_private *i915 = arg;
> >+    struct drm_i915_gem_object *obj;
> >+    const struct intel_rotation_plane_info planes[] = {
> >+            { .width = 1, .height = 1, .stride = 1 },
> >+            { .width = 3, .height = 5, .stride = 4 },
> >+            { .width = 5, .height = 3, .stride = 7 },
> >+            { .width = 6, .height = 4, .stride = 6 },
> 
> 4x6 stride 4 could be added if you were going for all combinations
> of wide/tall, equal stride and wider stride.

Just trying to pick some interesting ones. No rhyme or reason.

> >+            { }
> >+    }, *a, *b;
> >+    const unsigned int max_pages = 64;
> >+    int err = -ENOMEM;
> >+
> >+    /* Create VMA for many different combinations of planes and check
> >+     * that the page layout within the rotated VMA match our expectations.
> >+     */
> >+
> >+    obj = i915_gem_object_create_internal(i915, max_pages * PAGE_SIZE);
> >+    if (IS_ERR(obj))
> >+            goto err;
> >+
> >+    for (a = planes; a->width; a++) {
> >+            for (b = planes + ARRAY_SIZE(planes); b-- != planes; ) {
> >+                    struct i915_ggtt_view view;
> >+                    struct scatterlist *sg;
> >+                    unsigned int n, max_offset;
> >+
> >+                    max_offset = max(a->stride * a->height,
> >+                                     b->stride * b->height);
> 
> It shouldn't be min?
> 
> >+                    GEM_BUG_ON(max_offset >= max_pages);
> >+                    max_offset = max_pages - max_offset;

No, because it is inverted ^

> >+                    view.type = I915_GGTT_VIEW_ROTATED;
> >+                    view.rotated.plane[0] = *a;
> >+                    view.rotated.plane[1] = *b;
> 
> Single plane tests could be added as well.

There are. Second plane is set to {0}. That's the only way to do single
plane tests, as I was thinking second plane with a first plane would be
illegal?

> >+
> >+                    
> >for_each_prime_number_from(view.rotated.plane[0].offset, 0, max_offset) {
> >+                            
> >for_each_prime_number_from(view.rotated.plane[1].offset, 0, max_offset) {
> 
> I would try all offsets here and not only primes since it is super
> fast and more importantly more realistic.

I was worried about the combinatorial explosion. We could have upto
65536 checks for each pair of planes (currently x20).

> >+                                    struct i915_address_space *vm =
> >+                                            &i915->ggtt.base;
> >+                                    struct i915_vma *vma;
> >+
> >+                                    vma = i915_vma_instance(obj, vm, &view);
> >+                                    if (IS_ERR(vma)) {
> >+                                            err = PTR_ERR(vma);
> >+                                            goto err_object;
> >+                                    }
> >+
> >+                                    if (!i915_vma_is_ggtt(vma) ||
> >+                                        vma->vm != vm) {
> >+                                            pr_err("VMA is not in the 
> >GGTT!\n");
> >+                                            err = -EINVAL;
> >+                                            goto err_object;
> >+                                    }
> >+
> >+                                    if (memcmp(&vma->ggtt_view, &view, 
> >sizeof(view))) {
> 
> Just because rotation is the largest view! :) Need to use the "type" here.

I wasn't really sure the value in doing both memcmp() and
i915_vma_compare(). I think I'm just going to stick with
i915_vma_compare() only.
 
> >+                                            pr_err("VMA mismatch upon 
> >creation!\n");
> >+                                            err = -EINVAL;
> >+                                            goto err_object;
> >+                                    }
> >+
> >+                                    if (i915_vma_compare(vma,
> >+                                                         vma->vm,
> >+                                                         &vma->ggtt_view)) {
> >+                                            pr_err("VMA compmare failed 
> >with itself\n");

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

Reply via email to