Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_mmap: test for non pagealingned mmaps

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 08/04/2019 16:50, Ramalingam C wrote:
> Kernel supports the non page aligned mmaps. Hence this new subtest

s/Kernel/i915/ to make Chris happy. Maybe also expand with "by mistake" 
and clarify this applies only to CPU WC mmap flavour.

Unless Chris is objecting to testing this to start with?

> helps to maintain the sanity of such support.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>   tests/i915/gem_mmap.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/tests/i915/gem_mmap.c b/tests/i915/gem_mmap.c
> index 1f5348d9133b..f66d8b896268 100644
> --- a/tests/i915/gem_mmap.c
> +++ b/tests/i915/gem_mmap.c
> @@ -228,6 +228,21 @@ igt_main
>   		}
>   	}
>   
> +	igt_subtest("page_nonaligned") {
> +		struct drm_i915_gem_mmap arg = {
> +			.handle = gem_create(fd, PAGE_SIZE),
> +			.size = PAGE_SIZE - 5,
> +			.flags = I915_MMAP_WC,
> +		};
> +
> +		igt_debug("Trying to mmap page nonaligned size : %'"PRIu64"\n",
> +			  (unsigned long)arg.size);

PRIu64 does not match unsigned long on 32-bit I think.

But more importantly we normally don't bother with such debug messages 
especially when test is so trivial. So I'd just remove it.

> +		igt_assert_eq(mmap_ioctl(fd, &arg), 0);
> +
> +		gem_close(fd, arg.handle);
> +		munmap(from_user_pointer(arg.addr_ptr), PAGE_SIZE - 5);

Nitpick, add a local "const size_t mmap_size = PAGE_SIZE - 5" and then 
you don't have to repeat the value.

> +	}
> +
>   	igt_subtest("basic") {
>   		struct drm_i915_gem_mmap arg = {
>   			.handle = gem_create(fd, OBJECT_SIZE),
> 

Otherwise looks good to me.

Regards,

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