On 01/02/2017 14:55, Chris Wilson wrote:
On Wed, Feb 01, 2017 at 02:33:22PM +0000, Tvrtko Ursulin wrote:

[snip]

+               { }
+       }, *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 ^

I see.

+                       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?

Missed that.


+
+                       
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).

There is at least one even offset so OK. :)

+                                       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.

I'm OK with that. Wanted even to suggest dropping the is_ggtt test since that feels should happen in a more basic VMA creation test. But if such doesn't exist then it's fine.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to