Re: [igt-dev] [PATCH i-g-t] i915: Handle the case where legacy mmap is not available

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 24-06-2021 om 03:30 schreef Dixit, Ashutosh:
> On Tue, 11 May 2021 02:00:00 -0700, Maarten Lankhorst wrote:
>> @@ -84,10 +93,14 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>>  {
>> 	struct drm_i915_gem_mmap_gtt mmap_arg;
>> 	void *ptr;
>> +	int ret;
>>
>> 	memset(&mmap_arg, 0, sizeof(mmap_arg));
>> 	mmap_arg.handle = handle;
>> -	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg))
>> +	ret = igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg);
>> +	if (ret == -1 && errno == EOPNOTSUPP)
> What is the handler for DRM_IOCTL_I915_GEM_MMAP_GTT ioctl in i915 (in the
> kernel)? I am unable to find it so for now I am just assuming that it will
> just get routed to the handler for DRM_IOCTL_I915_GEM_MMAP and return
> EOPNOTSUPP for Gen12+ which is what the code above seems to be assuming.

tools/include/uapi/drm/i915_drm.h:#define DRM_IOCTL_I915_GEM_MMAP               DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
tools/include/uapi/drm/i915_drm.h:#define DRM_IOCTL_I915_GEM_MMAP_GTT   DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt)
tools/include/uapi/drm/i915_drm.h:#define DRM_IOCTL_I915_GEM_MMAP_OFFSET        DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)

I wasn't aware of that, even..

MMAP_GTT is MMAP_OFFSET with flags and extensions set to 0. Which means flags = I915_MMAP_OFFSET_GTT.

>> diff --git a/tests/i915/gem_mmap.c b/tests/i915/gem_mmap.c
>> index 7c36571c9bad..fdddd306acb5 100644
>> --- a/tests/i915/gem_mmap.c
>> +++ b/tests/i915/gem_mmap.c
>> @@ -154,8 +154,10 @@ igt_main
>> 	uint8_t buf[OBJECT_SIZE];
>> 	uint8_t *addr;
>>
>> -	igt_fixture
>> +	igt_fixture {
>> 		fd = drm_open_driver(DRIVER_INTEL);
>> +		igt_require(gem_has_legacy_mmap(fd));
>> +	}
>>
>> 	igt_subtest("bad-object") {
>> 		uint32_t real_handle = gem_create(fd, 4096);
>> diff --git a/tests/i915/gem_mmap_wc.c b/tests/i915/gem_mmap_wc.c
>> index 4a2192b30689..ad2d510fcf09 100644
>> --- a/tests/i915/gem_mmap_wc.c
>> +++ b/tests/i915/gem_mmap_wc.c
>> @@ -504,6 +504,7 @@ igt_main
>>
>> 	igt_fixture {
>> 		fd = drm_open_driver(DRIVER_INTEL);
>> +		igt_require(gem_has_legacy_mmap(fd));
> I believe we also need to add this igt_require in other places where
> DRM_IOCTL_I915_GEM_MMAP and DRM_IOCTL_I915_GEM_MMAP_GTT ioctls are being
> calld directly without calling the functions in gem_mman.*. Looks like we
> should at least add this igt_require to:
>
> * tests/i915/gem_tiled_wc.c, and
> * test/i915/gem_mmap_gtt.c
>
> The remaining references are in overlay/ and tools/ so maybe we can figure
> out what to do for these later?
>
> With the above igt_require added to the two files above, this is:
>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

Yeah looks like you're right. I assumed that on gen12+ it would work, because mmap_gtt was not working. I didn't see it was actually still implemented, because DG1 lacks GTT. :-)

I'll add the extra check.

~Maarten

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