[igt-dev] [PATCH i-g-t 1/5] lib/i915: Assert mmap size alignment

[igt-dev] [PATCH i-g-t 1/5] lib/i915: Assert mmap size alignment

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

mmap(2) mandates size is page aligned so check this in our wrappers.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/i915/gem_mman.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
index 3cf9a6bbdb31..084dbb3b3678 100644
--- a/lib/i915/gem_mman.c
+++ b/lib/i915/gem_mman.c
@@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
 	struct drm_i915_gem_mmap_gtt mmap_arg;
 	void *ptr;
 
+	igt_assert(!(size & 4095));
+
 	memset(&mmap_arg, 0, sizeof(mmap_arg));
 	mmap_arg.handle = handle;
 	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg))
@@ -162,6 +164,8 @@ static void
 {
 	struct drm_i915_gem_mmap arg;
 
+	igt_assert(!(size & 4095));
+
 	memset(&arg, 0, sizeof(arg));
 	arg.handle = handle;
 	arg.offset = offset;
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[PATCH i-g-t 1/5] lib/i915: Assert mmap size alignment

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

mmap(2) mandates size is page aligned so check this in our wrappers.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/i915/gem_mman.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
index 3cf9a6bbdb31..084dbb3b3678 100644
--- a/lib/i915/gem_mman.c
+++ b/lib/i915/gem_mman.c
@@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
 	struct drm_i915_gem_mmap_gtt mmap_arg;
 	void *ptr;
 
+	igt_assert(!(size & 4095));
+
 	memset(&mmap_arg, 0, sizeof(mmap_arg));
 	mmap_arg.handle = handle;
 	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg))
@@ -162,6 +164,8 @@ static void
 {
 	struct drm_i915_gem_mmap arg;
 
+	igt_assert(!(size & 4095));
+
 	memset(&arg, 0, sizeof(arg));
 	arg.handle = handle;
 	arg.offset = offset;
-- 
2.19.1

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

[Intel-gfx] [PATCH i-g-t 2/5] gem_wsim: Round mmap to page size

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

mmap(2) mandates size is page aligned.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 benchmarks/gem_wsim.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index 0a5abc08d8c2..57ceb983cf82 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -737,6 +737,7 @@ init_bb(struct w_step *w, unsigned int flags)
 {
 	const unsigned int arb_period =
 			get_bb_sz(w->preempt_us) / sizeof(uint32_t);
+	const unsigned int mmap_len = ALIGN(w->bb_sz, 4096);
 	unsigned int i;
 	uint32_t *ptr;
 
@@ -746,12 +747,12 @@ init_bb(struct w_step *w, unsigned int flags)
 	gem_set_domain(fd, w->bb_handle,
 		       I915_GEM_DOMAIN_WC, I915_GEM_DOMAIN_WC);
 
-	ptr = gem_mmap__wc(fd, w->bb_handle, 0, w->bb_sz, PROT_WRITE);
+	ptr = gem_mmap__wc(fd, w->bb_handle, 0, mmap_len, PROT_WRITE);
 
 	for (i = arb_period; i < w->bb_sz / sizeof(uint32_t); i += arb_period)
 		ptr[i] = 0x5 << 23; /* MI_ARB_CHK */
 
-	munmap(ptr, w->bb_sz);
+	munmap(ptr, mmap_len);
 }
 
 static void
@@ -771,7 +772,7 @@ terminate_bb(struct w_step *w, unsigned int flags)
 		batch_start -= 12 * sizeof(uint32_t);
 
 	mmap_start = rounddown(batch_start, PAGE_SIZE);
-	mmap_len = w->bb_sz - mmap_start;
+	mmap_len = ALIGN(w->bb_sz - mmap_start, PAGE_SIZE);
 
 	gem_set_domain(fd, w->bb_handle,
 		       I915_GEM_DOMAIN_WC, I915_GEM_DOMAIN_WC);
-- 
2.19.1

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

[PATCH i-g-t 2/5] gem_wsim: Round mmap to page size

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

mmap(2) mandates size is page aligned.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 benchmarks/gem_wsim.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index 0a5abc08d8c2..57ceb983cf82 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -737,6 +737,7 @@ init_bb(struct w_step *w, unsigned int flags)
 {
 	const unsigned int arb_period =
 			get_bb_sz(w->preempt_us) / sizeof(uint32_t);
+	const unsigned int mmap_len = ALIGN(w->bb_sz, 4096);
 	unsigned int i;
 	uint32_t *ptr;
 
@@ -746,12 +747,12 @@ init_bb(struct w_step *w, unsigned int flags)
 	gem_set_domain(fd, w->bb_handle,
 		       I915_GEM_DOMAIN_WC, I915_GEM_DOMAIN_WC);
 
-	ptr = gem_mmap__wc(fd, w->bb_handle, 0, w->bb_sz, PROT_WRITE);
+	ptr = gem_mmap__wc(fd, w->bb_handle, 0, mmap_len, PROT_WRITE);
 
 	for (i = arb_period; i < w->bb_sz / sizeof(uint32_t); i += arb_period)
 		ptr[i] = 0x5 << 23; /* MI_ARB_CHK */
 
-	munmap(ptr, w->bb_sz);
+	munmap(ptr, mmap_len);
 }
 
 static void
@@ -771,7 +772,7 @@ terminate_bb(struct w_step *w, unsigned int flags)
 		batch_start -= 12 * sizeof(uint32_t);
 
 	mmap_start = rounddown(batch_start, PAGE_SIZE);
-	mmap_len = w->bb_sz - mmap_start;
+	mmap_len = ALIGN(w->bb_sz - mmap_start, PAGE_SIZE);
 
 	gem_set_domain(fd, w->bb_handle,
 		       I915_GEM_DOMAIN_WC, I915_GEM_DOMAIN_WC);
-- 
2.19.1

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

Re: [Intel-gfx] [PATCH i-g-t 2/5] gem_wsim: Round mmap to page size

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:18:25)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> mmap(2) mandates size is page aligned.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  benchmarks/gem_wsim.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index 0a5abc08d8c2..57ceb983cf82 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -737,6 +737,7 @@ init_bb(struct w_step *w, unsigned int flags)
>  {
>         const unsigned int arb_period =
>                         get_bb_sz(w->preempt_us) / sizeof(uint32_t);
> +       const unsigned int mmap_len = ALIGN(w->bb_sz, 4096);
>         unsigned int i;
>         uint32_t *ptr;

You only need to do it for

ww->bb_sz = ALIGN(get_bb_sz(), 4096);

All batch lengths are start = ww->bb_sz - get_bb_sz() and so remain
correct. Right?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 2/5] gem_wsim: Round mmap to page size

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:18:25)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> mmap(2) mandates size is page aligned.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  benchmarks/gem_wsim.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index 0a5abc08d8c2..57ceb983cf82 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -737,6 +737,7 @@ init_bb(struct w_step *w, unsigned int flags)
>  {
>         const unsigned int arb_period =
>                         get_bb_sz(w->preempt_us) / sizeof(uint32_t);
> +       const unsigned int mmap_len = ALIGN(w->bb_sz, 4096);
>         unsigned int i;
>         uint32_t *ptr;

You only need to do it for

ww->bb_sz = ALIGN(get_bb_sz(), 4096);

All batch lengths are start = ww->bb_sz - get_bb_sz() and so remain
correct. Right?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 2/5] gem_wsim: Round mmap to page size

[Tvrtko Ursulin]

On 28/02/2019 14:25, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-02-28 14:18:25)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> mmap(2) mandates size is page aligned.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   benchmarks/gem_wsim.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
>> index 0a5abc08d8c2..57ceb983cf82 100644
>> --- a/benchmarks/gem_wsim.c
>> +++ b/benchmarks/gem_wsim.c
>> @@ -737,6 +737,7 @@ init_bb(struct w_step *w, unsigned int flags)
>>   {
>>          const unsigned int arb_period =
>>                          get_bb_sz(w->preempt_us) / sizeof(uint32_t);
>> +       const unsigned int mmap_len = ALIGN(w->bb_sz, 4096);
>>          unsigned int i;
>>          uint32_t *ptr;
> 
> You only need to do it for
> 
> ww->bb_sz = ALIGN(get_bb_sz(), 4096);
> 
> All batch lengths are start = ww->bb_sz - get_bb_sz() and so remain
> correct. Right?

I think so. I have one more assignment site of w->bb_sz in the upcoming 
code so it was 2 : 2 and I flipped a coin. Actually I wanted to make it 
explicit what I am fixing - the mmap size. But now you'll say batch size 
is also implicitly rounded.. oh well.. I prefer the explicit round up at 
mmap time.

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 2/5] gem_wsim: Round mmap to page size

[Tvrtko Ursulin]

On 28/02/2019 14:25, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-02-28 14:18:25)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> mmap(2) mandates size is page aligned.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   benchmarks/gem_wsim.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
>> index 0a5abc08d8c2..57ceb983cf82 100644
>> --- a/benchmarks/gem_wsim.c
>> +++ b/benchmarks/gem_wsim.c
>> @@ -737,6 +737,7 @@ init_bb(struct w_step *w, unsigned int flags)
>>   {
>>          const unsigned int arb_period =
>>                          get_bb_sz(w->preempt_us) / sizeof(uint32_t);
>> +       const unsigned int mmap_len = ALIGN(w->bb_sz, 4096);
>>          unsigned int i;
>>          uint32_t *ptr;
> 
> You only need to do it for
> 
> ww->bb_sz = ALIGN(get_bb_sz(), 4096);
> 
> All batch lengths are start = ww->bb_sz - get_bb_sz() and so remain
> correct. Right?

I think so. I have one more assignment site of w->bb_sz in the upcoming 
code so it was 2 : 2 and I flipped a coin. Actually I wanted to make it 
explicit what I am fixing - the mmap size. But now you'll say batch size 
is also implicitly rounded.. oh well.. I prefer the explicit round up at 
mmap time.

Regards,

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

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 2/5] gem_wsim: Round mmap to page size

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:41:23)
> 
> On 28/02/2019 14:25, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-02-28 14:18:25)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> mmap(2) mandates size is page aligned.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >>   benchmarks/gem_wsim.c | 7 ++++---
> >>   1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> >> index 0a5abc08d8c2..57ceb983cf82 100644
> >> --- a/benchmarks/gem_wsim.c
> >> +++ b/benchmarks/gem_wsim.c
> >> @@ -737,6 +737,7 @@ init_bb(struct w_step *w, unsigned int flags)
> >>   {
> >>          const unsigned int arb_period =
> >>                          get_bb_sz(w->preempt_us) / sizeof(uint32_t);
> >> +       const unsigned int mmap_len = ALIGN(w->bb_sz, 4096);
> >>          unsigned int i;
> >>          uint32_t *ptr;
> > 
> > You only need to do it for
> > 
> > ww->bb_sz = ALIGN(get_bb_sz(), 4096);
> > 
> > All batch lengths are start = ww->bb_sz - get_bb_sz() and so remain
> > correct. Right?
> 
> I think so. I have one more assignment site of w->bb_sz in the upcoming 
> code so it was 2 : 2 and I flipped a coin. Actually I wanted to make it 
> explicit what I am fixing - the mmap size. But now you'll say batch size 
> is also implicitly rounded.. oh well.. I prefer the explicit round up at 
> mmap time.

Ok,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 2/5] gem_wsim: Round mmap to page size

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:41:23)
> 
> On 28/02/2019 14:25, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-02-28 14:18:25)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> mmap(2) mandates size is page aligned.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >>   benchmarks/gem_wsim.c | 7 ++++---
> >>   1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> >> index 0a5abc08d8c2..57ceb983cf82 100644
> >> --- a/benchmarks/gem_wsim.c
> >> +++ b/benchmarks/gem_wsim.c
> >> @@ -737,6 +737,7 @@ init_bb(struct w_step *w, unsigned int flags)
> >>   {
> >>          const unsigned int arb_period =
> >>                          get_bb_sz(w->preempt_us) / sizeof(uint32_t);
> >> +       const unsigned int mmap_len = ALIGN(w->bb_sz, 4096);
> >>          unsigned int i;
> >>          uint32_t *ptr;
> > 
> > You only need to do it for
> > 
> > ww->bb_sz = ALIGN(get_bb_sz(), 4096);
> > 
> > All batch lengths are start = ww->bb_sz - get_bb_sz() and so remain
> > correct. Right?
> 
> I think so. I have one more assignment site of w->bb_sz in the upcoming 
> code so it was 2 : 2 and I flipped a coin. Actually I wanted to make it 
> explicit what I am fixing - the mmap size. But now you'll say batch size 
> is also implicitly rounded.. oh well.. I prefer the explicit round up at 
> mmap time.

Ok,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 3/5] gem_wsim: Remove some unused struct members

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We do not bother explicitly unmapping memory on exit so no need to store
address and size in the workload step struct.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 benchmarks/gem_wsim.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index 57ceb983cf82..afb9644dd7f0 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -131,7 +131,6 @@ struct w_step
 	struct drm_i915_gem_relocation_entry reloc[4];
 	unsigned long bb_sz;
 	uint32_t bb_handle;
-	uint32_t *mapped_batch;
 	uint32_t *seqno_value;
 	uint32_t *seqno_address;
 	uint32_t *rt0_value;
@@ -139,7 +138,6 @@ struct w_step
 	uint32_t *rt1_address;
 	uint32_t *latch_value;
 	uint32_t *latch_address;
-	unsigned int mapped_len;
 };
 
 DECLARE_EWMA(uint64_t, rt, 4, 2)
@@ -824,9 +822,6 @@ terminate_bb(struct w_step *w, unsigned int flags)
 	}
 
 	*cs = bbe;
-
-	w->mapped_batch = ptr;
-	w->mapped_len = mmap_len;
 }
 
 static const unsigned int eb_engine_map[NUM_ENGINES] = {
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[PATCH i-g-t 3/5] gem_wsim: Remove some unused struct members

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We do not bother explicitly unmapping memory on exit so no need to store
address and size in the workload step struct.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 benchmarks/gem_wsim.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index 57ceb983cf82..afb9644dd7f0 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -131,7 +131,6 @@ struct w_step
 	struct drm_i915_gem_relocation_entry reloc[4];
 	unsigned long bb_sz;
 	uint32_t bb_handle;
-	uint32_t *mapped_batch;
 	uint32_t *seqno_value;
 	uint32_t *seqno_address;
 	uint32_t *rt0_value;
@@ -139,7 +138,6 @@ struct w_step
 	uint32_t *rt1_address;
 	uint32_t *latch_value;
 	uint32_t *latch_address;
-	unsigned int mapped_len;
 };
 
 DECLARE_EWMA(uint64_t, rt, 4, 2)
@@ -824,9 +822,6 @@ terminate_bb(struct w_step *w, unsigned int flags)
 	}
 
 	*cs = bbe;
-
-	w->mapped_batch = ptr;
-	w->mapped_len = mmap_len;
 }
 
 static const unsigned int eb_engine_map[NUM_ENGINES] = {
-- 
2.19.1

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

Re: [igt-dev] [PATCH i-g-t 3/5] gem_wsim: Remove some unused struct members

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:18:26)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> We do not bother explicitly unmapping memory on exit so no need to store
> address and size in the workload step struct.

If compiler happy, and I'm happy about leaving the mmap dangling for the
kernel to clean up,

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/5] gem_wsim: Remove some unused struct members

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:18:26)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> We do not bother explicitly unmapping memory on exit so no need to store
> address and size in the workload step struct.

If compiler happy, and I'm happy about leaving the mmap dangling for the
kernel to clean up,

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 4/5] autoconf: Silence void pointer arithmetic warnings

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Meson build does it so make the two symmetrical in this respect.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 configure.ac | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure.ac b/configure.ac
index 4f55ea5d0f89..210e2c57df55 100644
--- a/configure.ac
+++ b/configure.ac
@@ -256,6 +256,8 @@ AC_ARG_ENABLE(werror,
 			     [Fail on warnings]),
 	      [], [enable_werror=no])
 
+CWARNFLAGS="$CWARNFLAGS -Wno-pointer-arith"
+
 if test "x$enable_debug" = xyes; then
 	AS_COMPILER_FLAG([-g3], [DEBUG_CFLAGS="-g3"], [DEBUG_CFLAGS="-g"])
 	AS_COMPILER_FLAG([-Og], [DEBUG_CFLAGS+=" -Og -Wno-maybe-uninitialized"], # disable maybe-uninitialized due to false positives
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[PATCH i-g-t 4/5] autoconf: Silence void pointer arithmetic warnings

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Meson build does it so make the two symmetrical in this respect.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 configure.ac | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure.ac b/configure.ac
index 4f55ea5d0f89..210e2c57df55 100644
--- a/configure.ac
+++ b/configure.ac
@@ -256,6 +256,8 @@ AC_ARG_ENABLE(werror,
 			     [Fail on warnings]),
 	      [], [enable_werror=no])
 
+CWARNFLAGS="$CWARNFLAGS -Wno-pointer-arith"
+
 if test "x$enable_debug" = xyes; then
 	AS_COMPILER_FLAG([-g3], [DEBUG_CFLAGS="-g3"], [DEBUG_CFLAGS="-g"])
 	AS_COMPILER_FLAG([-Og], [DEBUG_CFLAGS+=" -Og -Wno-maybe-uninitialized"], # disable maybe-uninitialized due to false positives
-- 
2.19.1

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

Re: [igt-dev] [PATCH i-g-t 4/5] autoconf: Silence void pointer arithmetic warnings

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:18:27)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Meson build does it so make the two symmetrical in this respect.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  configure.ac | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 4f55ea5d0f89..210e2c57df55 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -256,6 +256,8 @@ AC_ARG_ENABLE(werror,
>                              [Fail on warnings]),
>               [], [enable_werror=no])
>  
> +CWARNFLAGS="$CWARNFLAGS -Wno-pointer-arith"

\o/

> +
>  if test "x$enable_debug" = xyes; then
>         AS_COMPILER_FLAG([-g3], [DEBUG_CFLAGS="-g3"], [DEBUG_CFLAGS="-g"])
>         AS_COMPILER_FLAG([-Og], [DEBUG_CFLAGS+=" -Og -Wno-maybe-uninitialized"], # disable maybe-uninitialized due to false positives
> -- 

But shouldn't we be using something like the above to verify the flag
exists?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/5] autoconf: Silence void pointer arithmetic warnings

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:18:27)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Meson build does it so make the two symmetrical in this respect.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  configure.ac | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 4f55ea5d0f89..210e2c57df55 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -256,6 +256,8 @@ AC_ARG_ENABLE(werror,
>                              [Fail on warnings]),
>               [], [enable_werror=no])
>  
> +CWARNFLAGS="$CWARNFLAGS -Wno-pointer-arith"

\o/

> +
>  if test "x$enable_debug" = xyes; then
>         AS_COMPILER_FLAG([-g3], [DEBUG_CFLAGS="-g3"], [DEBUG_CFLAGS="-g"])
>         AS_COMPILER_FLAG([-Og], [DEBUG_CFLAGS+=" -Og -Wno-maybe-uninitialized"], # disable maybe-uninitialized due to false positives
> -- 

But shouldn't we be using something like the above to verify the flag
exists?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 4/5] autoconf: Silence void pointer arithmetic warnings

[Tvrtko Ursulin]

On 28/02/2019 14:31, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-02-28 14:18:27)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Meson build does it so make the two symmetrical in this respect.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   configure.ac | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 4f55ea5d0f89..210e2c57df55 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -256,6 +256,8 @@ AC_ARG_ENABLE(werror,
>>                               [Fail on warnings]),
>>                [], [enable_werror=no])
>>   
>> +CWARNFLAGS="$CWARNFLAGS -Wno-pointer-arith"
> 
> \o/

Maybe I am only testing to see who still uses automake. :))

>> +
>>   if test "x$enable_debug" = xyes; then
>>          AS_COMPILER_FLAG([-g3], [DEBUG_CFLAGS="-g3"], [DEBUG_CFLAGS="-g"])
>>          AS_COMPILER_FLAG([-Og], [DEBUG_CFLAGS+=" -Og -Wno-maybe-uninitialized"], # disable maybe-uninitialized due to false positives
>> -- 
> 
> But shouldn't we be using something like the above to verify the flag
> exists?

Definitely true.

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/5] autoconf: Silence void pointer arithmetic warnings

[Tvrtko Ursulin]

On 28/02/2019 14:31, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-02-28 14:18:27)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Meson build does it so make the two symmetrical in this respect.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   configure.ac | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 4f55ea5d0f89..210e2c57df55 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -256,6 +256,8 @@ AC_ARG_ENABLE(werror,
>>                               [Fail on warnings]),
>>                [], [enable_werror=no])
>>   
>> +CWARNFLAGS="$CWARNFLAGS -Wno-pointer-arith"
> 
> \o/

Maybe I am only testing to see who still uses automake. :))

>> +
>>   if test "x$enable_debug" = xyes; then
>>          AS_COMPILER_FLAG([-g3], [DEBUG_CFLAGS="-g3"], [DEBUG_CFLAGS="-g"])
>>          AS_COMPILER_FLAG([-Og], [DEBUG_CFLAGS+=" -Og -Wno-maybe-uninitialized"], # disable maybe-uninitialized due to false positives
>> -- 
> 
> But shouldn't we be using something like the above to verify the flag
> exists?

Definitely true.

Regards,

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

[Intel-gfx] [PATCH i-g-t 5/5] tests/i915/pm_rc6_residency: Fix linking

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Probably a leftover from test renames:

  tests/Makefile.am:134: warning: variable 'pm_rc6_residency_LDADD' is defined but no program or
  tests/Makefile.am:134: library has 'pm_rc6_residency' as canonical name (possible typo)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c5dd210c7163..80bc5f92a13b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -131,7 +131,7 @@ perf_pmu_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
 gem_eio_LDADD = $(LDADD) -lrt
 gem_wait_LDADD = $(LDADD) -lrt
 kms_flip_LDADD = $(LDADD) -lrt -lpthread
-pm_rc6_residency_LDADD = $(LDADD) -lrt
+i915_pm_rc6_residency_LDADD = $(LDADD) -lrt
 
 prime_nv_test_CFLAGS = $(AM_CFLAGS) $(DRM_NOUVEAU_CFLAGS)
 prime_nv_test_LDADD = $(LDADD) $(DRM_NOUVEAU_LIBS)
-- 
2.19.1

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

[PATCH i-g-t 5/5] tests/i915/pm_rc6_residency: Fix linking

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Probably a leftover from test renames:

  tests/Makefile.am:134: warning: variable 'pm_rc6_residency_LDADD' is defined but no program or
  tests/Makefile.am:134: library has 'pm_rc6_residency' as canonical name (possible typo)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c5dd210c7163..80bc5f92a13b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -131,7 +131,7 @@ perf_pmu_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
 gem_eio_LDADD = $(LDADD) -lrt
 gem_wait_LDADD = $(LDADD) -lrt
 kms_flip_LDADD = $(LDADD) -lrt -lpthread
-pm_rc6_residency_LDADD = $(LDADD) -lrt
+i915_pm_rc6_residency_LDADD = $(LDADD) -lrt
 
 prime_nv_test_CFLAGS = $(AM_CFLAGS) $(DRM_NOUVEAU_CFLAGS)
 prime_nv_test_LDADD = $(LDADD) $(DRM_NOUVEAU_LIBS)
-- 
2.19.1

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

Re: [igt-dev] [PATCH i-g-t 5/5] tests/i915/pm_rc6_residency: Fix linking

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:18:28)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Probably a leftover from test renames:
> 
>   tests/Makefile.am:134: warning: variable 'pm_rc6_residency_LDADD' is defined but no program or
>   tests/Makefile.am:134: library has 'pm_rc6_residency' as canonical name (possible typo)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tests/Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index c5dd210c7163..80bc5f92a13b 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -131,7 +131,7 @@ perf_pmu_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
>  gem_eio_LDADD = $(LDADD) -lrt
>  gem_wait_LDADD = $(LDADD) -lrt
>  kms_flip_LDADD = $(LDADD) -lrt -lpthread
> -pm_rc6_residency_LDADD = $(LDADD) -lrt
> +i915_pm_rc6_residency_LDADD = $(LDADD) -lrt

Have we not snuck -lrt into the core library yet, pretty sure libigt.la
now includes clock_gettime() and so must be pulling it in already?

i.e. can we just drop the above line?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/5] tests/i915/pm_rc6_residency: Fix linking

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:18:28)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Probably a leftover from test renames:
> 
>   tests/Makefile.am:134: warning: variable 'pm_rc6_residency_LDADD' is defined but no program or
>   tests/Makefile.am:134: library has 'pm_rc6_residency' as canonical name (possible typo)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tests/Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index c5dd210c7163..80bc5f92a13b 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -131,7 +131,7 @@ perf_pmu_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
>  gem_eio_LDADD = $(LDADD) -lrt
>  gem_wait_LDADD = $(LDADD) -lrt
>  kms_flip_LDADD = $(LDADD) -lrt -lpthread
> -pm_rc6_residency_LDADD = $(LDADD) -lrt
> +i915_pm_rc6_residency_LDADD = $(LDADD) -lrt

Have we not snuck -lrt into the core library yet, pretty sure libigt.la
now includes clock_gettime() and so must be pulling it in already?

i.e. can we just drop the above line?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 5/5] tests/i915/pm_rc6_residency: Fix linking

[Tvrtko Ursulin]

On 28/02/2019 14:33, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-02-28 14:18:28)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Probably a leftover from test renames:
>>
>>    tests/Makefile.am:134: warning: variable 'pm_rc6_residency_LDADD' is defined but no program or
>>    tests/Makefile.am:134: library has 'pm_rc6_residency' as canonical name (possible typo)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   tests/Makefile.am | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index c5dd210c7163..80bc5f92a13b 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -131,7 +131,7 @@ perf_pmu_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
>>   gem_eio_LDADD = $(LDADD) -lrt
>>   gem_wait_LDADD = $(LDADD) -lrt
>>   kms_flip_LDADD = $(LDADD) -lrt -lpthread
>> -pm_rc6_residency_LDADD = $(LDADD) -lrt
>> +i915_pm_rc6_residency_LDADD = $(LDADD) -lrt
> 
> Have we not snuck -lrt into the core library yet, pretty sure libigt.la
> now includes clock_gettime() and so must be pulling it in already?
> 
> i.e. can we just drop the above line?

Looks like it. And many more in this case.

Regards,

Tvrtko

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/5] tests/i915/pm_rc6_residency: Fix linking

[Tvrtko Ursulin]

On 28/02/2019 14:33, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-02-28 14:18:28)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Probably a leftover from test renames:
>>
>>    tests/Makefile.am:134: warning: variable 'pm_rc6_residency_LDADD' is defined but no program or
>>    tests/Makefile.am:134: library has 'pm_rc6_residency' as canonical name (possible typo)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   tests/Makefile.am | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index c5dd210c7163..80bc5f92a13b 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -131,7 +131,7 @@ perf_pmu_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
>>   gem_eio_LDADD = $(LDADD) -lrt
>>   gem_wait_LDADD = $(LDADD) -lrt
>>   kms_flip_LDADD = $(LDADD) -lrt -lpthread
>> -pm_rc6_residency_LDADD = $(LDADD) -lrt
>> +i915_pm_rc6_residency_LDADD = $(LDADD) -lrt
> 
> Have we not snuck -lrt into the core library yet, pretty sure libigt.la
> now includes clock_gettime() and so must be pulling it in already?
> 
> i.e. can we just drop the above line?

Looks like it. And many more in this case.

Regards,

Tvrtko

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

Re: [igt-dev] [PATCH i-g-t 1/5] lib/i915: Assert mmap size alignment

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:18:24)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> mmap(2) mandates size is page aligned so check this in our wrappers.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/i915/gem_mman.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> index 3cf9a6bbdb31..084dbb3b3678 100644
> --- a/lib/i915/gem_mman.c
> +++ b/lib/i915/gem_mman.c
> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>         struct drm_i915_gem_mmap_gtt mmap_arg;
>         void *ptr;
>  
> +       igt_assert(!(size & 4095));

No, we don't filter what the kernel gets, not in the ioctl wrapper
itself.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/5] lib/i915: Assert mmap size alignment

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-28 14:18:24)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> mmap(2) mandates size is page aligned so check this in our wrappers.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/i915/gem_mman.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> index 3cf9a6bbdb31..084dbb3b3678 100644
> --- a/lib/i915/gem_mman.c
> +++ b/lib/i915/gem_mman.c
> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>         struct drm_i915_gem_mmap_gtt mmap_arg;
>         void *ptr;
>  
> +       igt_assert(!(size & 4095));

No, we don't filter what the kernel gets, not in the ioctl wrapper
itself.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/5] lib/i915: Assert mmap size alignment

[Tvrtko Ursulin]

On 28/02/2019 14:24, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-02-28 14:18:24)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> mmap(2) mandates size is page aligned so check this in our wrappers.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   lib/i915/gem_mman.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
>> index 3cf9a6bbdb31..084dbb3b3678 100644
>> --- a/lib/i915/gem_mman.c
>> +++ b/lib/i915/gem_mman.c
>> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>>          struct drm_i915_gem_mmap_gtt mmap_arg;
>>          void *ptr;
>>   
>> +       igt_assert(!(size & 4095));
> 
> No, we don't filter what the kernel gets, not in the ioctl wrapper
> itself.

I know, but I could flip a coin on these kind of decisions. (Sometimes 
we open code for such ABI tests.) Move it to non-double underscore flavours?

Regards,

Tvrtko

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/5] lib/i915: Assert mmap size alignment

[Tvrtko Ursulin]

On 28/02/2019 14:24, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-02-28 14:18:24)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> mmap(2) mandates size is page aligned so check this in our wrappers.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   lib/i915/gem_mman.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
>> index 3cf9a6bbdb31..084dbb3b3678 100644
>> --- a/lib/i915/gem_mman.c
>> +++ b/lib/i915/gem_mman.c
>> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>>          struct drm_i915_gem_mmap_gtt mmap_arg;
>>          void *ptr;
>>   
>> +       igt_assert(!(size & 4095));
> 
> No, we don't filter what the kernel gets, not in the ioctl wrapper
> itself.

I know, but I could flip a coin on these kind of decisions. (Sometimes 
we open code for such ABI tests.) Move it to non-double underscore flavours?

Regards,

Tvrtko

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

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/5] lib/i915: Assert mmap size alignment

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/5] lib/i915: Assert mmap size alignment
URL   : https://patchwork.freedesktop.org/series/57345/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5674 -> IGTPW_2535
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2535 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2535, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57345/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_2535:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_psr@primary_mmap_gtt:
    - fi-kbl-7560u:       PASS -> FAIL
    - fi-whl-u:           PASS -> FAIL

  
Known issues
------------

  Here are the changes found in IGTPW_2535 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-u2:          PASS -> INCOMPLETE [fdo#108569]

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-hsw-peppy:       NOTRUN -> SKIP [fdo#109271] +46

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       NOTRUN -> DMESG-FAIL [fdo#102614] / [fdo#107814]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720] / [fdo#109799]

  
#### Possible fixes ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       DMESG-WARN [fdo#108965] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107814]: https://bugs.freedesktop.org/show_bug.cgi?id=107814
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#109799]: https://bugs.freedesktop.org/show_bug.cgi?id=109799


Participating hosts (44 -> 40)
------------------------------

  Additional (1): fi-hsw-peppy 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


Build changes
-------------

    * IGT: IGT_4863 -> IGTPW_2535

  CI_DRM_5674: 71bb3bfb61fb58f93f8b09e6ad576a403cd7752c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2535: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2535/
  IGT_4863: 0f0db14e7f4ec41251ca156d7cb5b8d531a38006 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2535/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev