<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof ContentPasted0 ContentPasted1 ContentPasted2 ContentPasted3">
> Hi Fei,
<div class="ContentPasted0">></div>
<div class="ContentPasted0">> On Wed, May 24, 2023 at 08:56:10AM -0700, fei.yang@intel.com wrote:</div>
<div class="ContentPasted0">>> From: Fei Yang <fei.yang@intel.com></div>
<div class="ContentPasted0">>></div>
<div class="ContentPasted0">>> Gem_create uAPI has been extended with capability of setting PAT index.</div>
<div class="ContentPasted0">>> Add a test case to exercise it.</div>
<div class="ContentPasted0">></div>
<div class="ContentPasted0">> As we discussed offline, can we:</div>
<div class="ContentPasted0">></div>
<div class="ContentPasted0">> /PAT/PAT (Page Attribute Table)/</div>
<div><br class="ContentPasted0">
</div>
<div class="ContentPasted0">I did explain "PAT is a page attribute" below. If that's not good enough I</div>
<div class="ContentPasted0">have no objection to add what you proposed above.</div>
<div><br class="ContentPasted0">
</div>
<div class="ContentPasted0">> But I think Kamil can fix this before pushing it.</div>
<div class="ContentPasted0">></div>
<div class="ContentPasted0">>> PAT index is a page attribute that gets programmed into Page Table</div>
<div class="ContentPasted0">>> Entry (PTE) for caching policy control. Please refer to kernel patch</div>
<div class="ContentPasted0">>> for implementation details.</div>
<div class="ContentPasted0">>></div>
<div class="ContentPasted0">>> https://patchwork.freedesktop.org/series/116870/</div>
<div class="ContentPasted0">>></div>
<div class="ContentPasted0">>> Signed-off-by: Fei Yang <fei.yang@intel.com></div>
<div class="ContentPasted0">>> ---</div>
<div class="ContentPasted0">>> lib/i915/i915_drm_local.h | 34 +++++++++++++++++++++++</div>
<div class="ContentPasted0">>> tests/i915/gem_create.c | 58 +++++++++++++++++++++++++++++++++++++++</div>
<div class="ContentPasted0">>> 2 files changed, 92 insertions(+)</div>
<div class="ContentPasted0">>></div>
<div class="ContentPasted0">>> diff --git a/lib/i915/i915_drm_local.h b/lib/i915/i915_drm_local.h</div>
<div class="ContentPasted0">>> index bb2ebef3..0f47578c 100644</div>
<div class="ContentPasted0">>> --- a/lib/i915/i915_drm_local.h</div>
<div class="ContentPasted0">>> +++ b/lib/i915/i915_drm_local.h</div>
<div class="ContentPasted0">>> @@ -41,6 +41,40 @@ extern "C" {</div>
<div class="ContentPasted0">>> #define __I915_PMU_RC6_RESIDENCY(gt) ___I915_PMU_OTHER(gt, 3)</div>
<div class="ContentPasted0">>> #define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ___I915_PMU_OTHER(gt, 4)</div>
<div class="ContentPasted0">>></div>
<div class="ContentPasted0">>> +#define I915_GEM_CREATE_EXT_SET_PAT 2</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> +/**</div>
<div class="ContentPasted0">>> + * struct drm_i915_gem_create_ext_set_pat - The</div>
>> + * I915_GEM_CREATE_EXT_SET_PAT extension.
<div class="ContentPasted1">>> + *</div>
<div class="ContentPasted1">>> + * If this extension is provided, the specified caching policy (PAT index) is</div>
<div class="ContentPasted1">>> + * applied to the buffer object.</div>
<div class="ContentPasted1">>> + *</div>
<div class="ContentPasted1">>> + * Below is an example on how to create an object with specific caching policy:</div>
<div class="ContentPasted1">>> + *</div>
<div class="ContentPasted1">>> + * .. code-block:: C</div>
<div class="ContentPasted1">>> + *</div>
<div class="ContentPasted1">>> + * struct drm_i915_gem_create_ext_set_pat set_pat_ext = {</div>
<div class="ContentPasted1">>> + * .base = { .name = I915_GEM_CREATE_EXT_SET_PAT },</div>
<div class="ContentPasted1">>> + * .pat_index = 0,</div>
<div class="ContentPasted1">>> + * };</div>
<div class="ContentPasted1">>> + * struct drm_i915_gem_create_ext create_ext = {</div>
<div class="ContentPasted1">>> + * .size = PAGE_SIZE,</div>
<div class="ContentPasted1">>> + * .extensions = (uintptr_t)&set_pat_ext,</div>
<div class="ContentPasted1">>> + * };</div>
<div class="ContentPasted1">>> + *</div>
<div class="ContentPasted1">>> + * int err = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext);</div>
<div class="ContentPasted1">>> + * if (err) ...</div>
<div class="ContentPasted1">>> + */</div>
<div class="ContentPasted1">>> +struct drm_i915_gem_create_ext_set_pat {</div>
<div class="ContentPasted1">>> + /** @base: Extension link. See struct i915_user_extension. */</div>
<div class="ContentPasted1">>> + struct i915_user_extension base;</div>
<div class="ContentPasted1">>> + /** @pat_index: PAT index to be set */</div>
<div class="ContentPasted1">>> + __u32 pat_index;</div>
<div class="ContentPasted1">>> + /** @rsvd: reserved for future use */</div>
<div class="ContentPasted1">>> + __u32 rsvd;</div>
<div class="ContentPasted1">>> +};</div>
<div class="ContentPasted1">>> +</div>
<div class="ContentPasted1">>> #if defined(__cplusplus)</div>
<div class="ContentPasted1">>> }</div>
<div class="ContentPasted1">>> #endif</div>
<div class="ContentPasted1">>> diff --git a/tests/i915/gem_create.c b/tests/i915/gem_create.c</div>
<div class="ContentPasted1">>> index fc991894..2e0c0ab9 100644</div>
<div class="ContentPasted1">>> --- a/tests/i915/gem_create.c</div>
<div class="ContentPasted1">>> +++ b/tests/i915/gem_create.c</div>
<div class="ContentPasted1">>> @@ -587,6 +587,60 @@ static void create_ext_placement_each(int fd)</div>
<div class="ContentPasted1">>> free(regions);</div>
<div class="ContentPasted1">>> }</div>
<div class="ContentPasted1">>></div>
>> +/**
<div class="ContentPasted2">>> + * TEST: GEM create uAPI</div>
<div class="ContentPasted2">>> + * Category: Infrastructure</div>
<div class="ContentPasted2">>> + * Description: Tests the DRM_IOCTL_I915_GEM_CREATE_EXT ioctl.</div>
<div class="ContentPasted2">>> + * Feature: core</div>
<div class="ContentPasted2">>> + * Functionality: Create GEM object with specific PAT index</div>
<div class="ContentPasted2">>> + * Run type: BAT</div>
<div class="ContentPasted2">>> + * Sub-category: i915</div>
<div class="ContentPasted2">>> + * Test category: GEM_Legacy</div>
<div class="ContentPasted2">>> + *</div>
<div class="ContentPasted2">>> + * SUBTEST: create-ext-set-pat</div>
<div class="ContentPasted2">>> + */</div>
<div class="ContentPasted2">>> +static void create_ext_set_pat(int fd)</div>
<div class="ContentPasted2">>> +{</div>
<div class="ContentPasted2">>> + struct drm_i915_gem_create_ext_set_pat setparam_pat = {</div>
<div class="ContentPasted2">>> + .base = { .name = I915_GEM_CREATE_EXT_SET_PAT },</div>
<div class="ContentPasted2">>> + .pat_index = 0,</div>
<div class="ContentPasted2">>> + };</div>
<div class="ContentPasted2">>> + struct drm_i915_gem_create_ext_set_pat setparam_inv_pat = {</div>
<div class="ContentPasted2">>> + .base = { .name = I915_GEM_CREATE_EXT_SET_PAT },</div>
<div class="ContentPasted2">>> + .pat_index = 65,</div>
<div class="ContentPasted2">>> + };</div>
<div class="ContentPasted2">>> + struct drm_i915_gem_caching arg;</div>
<div class="ContentPasted2">>> + uint64_t size;</div>
<div class="ContentPasted2">>> + uint32_t handle;</div>
<div class="ContentPasted2">>> + int ret;</div>
<div class="ContentPasted2">>> +</div>
<div class="ContentPasted2">>> + size = PAGE_SIZE;</div>
<div class="ContentPasted2">>> +</div>
<div class="ContentPasted2">>> + ret = __gem_create_ext(fd, &size, 0, &handle, &setparam_pat.base);</div>
<div class="ContentPasted2">></div>
<div class="ContentPasted2">> Tvrtko was talking about a -EFAULT path that can come only from</div>
<div class="ContentPasted2">> the use extention, unless I missed any other path.</div>
<div class="ContentPasted2">></div>
<div class="ContentPasted2">> I'm OK if we skip it as it would triggered by setting:</div>
<div class="ContentPasted2">></div>
<div class="ContentPasted2">> "name = <something_random>"</div>
<div class="ContentPasted2">></div>
<div class="ContentPasted2">> right?</div>
<div><br class="ContentPasted2">
</div>
<div class="ContentPasted2">The point is that we want specifically check if a failure here is due to</div>
kernel not up-to-date. Otherwise any none-zero return should be</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof ContentPasted0 ContentPasted1 ContentPasted2 ContentPasted3 ContentPasted4">
treated as failure.
<div><br class="ContentPasted3">
</div>
<div class="ContentPasted3">> In any case:</div>
<div class="ContentPasted3">></div>
<div class="ContentPasted3">> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com></div>
<div class="ContentPasted3">></div>
<div class="ContentPasted3">> Andi</div>
<div class="ContentPasted3">></div>
<div class="ContentPasted3">>> + /*</div>
<div class="ContentPasted3">>> + * With a valid PAT index specified, returning -EINVAL here</div>
<div class="ContentPasted3">>> + * indicates set_pat extension is not supported</div>
<div class="ContentPasted3">>> + */</div>
<div class="ContentPasted3">>> + if (ret == -EINVAL)</div>
<div class="ContentPasted3">>> + igt_skip("I915_GEM_CREATE_EXT_SET_PAT is not supported\n");</div>
<div class="ContentPasted3">>> + igt_assert(ret == 0);</div>
<div class="ContentPasted3">>> +</div>
<div class="ContentPasted3">>> + /* {set|get}_caching ioctl should fail for objects created with set_pat */</div>
<div class="ContentPasted3">>> + igt_assert_eq(__gem_set_caching(fd, handle, 1), -EOPNOTSUPP);</div>
<div class="ContentPasted3">>> +</div>
<div class="ContentPasted3">>> + memset(&arg, 0, sizeof(arg));</div>
<div class="ContentPasted3">>> + arg.handle = handle;</div>
<div class="ContentPasted3">>> + igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_GET_CACHING, &arg) < 0 &&</div>
<div class="ContentPasted3">>> + errno == EOPNOTSUPP);</div>
<div class="ContentPasted3">>> +</div>
<div class="ContentPasted3">>> + /* gem_create should fail with -EINVAL if invalid pat index specified */</div>
<div class="ContentPasted3">>> + igt_assert_eq(__gem_create_ext(fd, &size, 0, &handle, &setparam_inv_pat.base),</div>
<div class="ContentPasted3">>> + -EINVAL);</div>
<div class="ContentPasted3">>> +</div>
<div class="ContentPasted3">>> + gem_close(fd, handle);</div>
<div class="ContentPasted3">>> +}</div>
<div class="ContentPasted3">>> +</div>
<div class="ContentPasted3">>> static bool supports_needs_cpu_access(int fd)</div>
<div class="ContentPasted3">>> {</div>
<div class="ContentPasted3">>> struct drm_i915_gem_memory_class_instance regions[] = {</div>
<div class="ContentPasted3">>> @@ -947,6 +1001,10 @@ igt_main</div>
<div class="ContentPasted3">>> igt_subtest("create-ext-placement-all")</div>
<div class="ContentPasted3">>> create_ext_placement_all(fd);</div>
<div class="ContentPasted3">>></div>
<div class="ContentPasted3">>> + igt_describe("Validate basic creation of objects with PAT cache setting.");</div>
<div class="ContentPasted3">>> + igt_subtest("create-ext-set-pat")</div>
>> + create_ext_set_pat(fd);
<div class="ContentPasted4">>> +</div>
<div class="ContentPasted4">>> igt_describe("Verify the basic functionally and expected ABI contract around I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS");</div>
<div class="ContentPasted4">>> igt_subtest("create-ext-cpu-access-sanity-check") {</div>
<div class="ContentPasted4">>> igt_require(supports_needs_cpu_access(fd));</div>
<div class="ContentPasted4">>> --</div>
<div class="ContentPasted4">>> 2.25.1</div>
<br>
</div>
</body>
</html>