<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">
> On 16/05/2023 19:11, fei.yang@intel.com wrote:
<div class="ContentPasted0">>> From: Fei Yang <fei.yang@intel.com></div>
<div class="ContentPasted0">>></div>
<div class="ContentPasted0">>> To comply with the design that buffer objects shall have immutable</div>
<div class="ContentPasted0">>> cache setting through out their life cycle, {set, get}_caching ioctl's</div>
<div class="ContentPasted0">>> are no longer supported from MTL onward. With that change caching</div>
<div class="ContentPasted0">>> policy can only be set at object creation time. The current code</div>
<div class="ContentPasted0">>> applies a default (platform dependent) cache setting for all objects.</div>
<div class="ContentPasted0">>> However this is not optimal for performance tuning. The patch extends</div>
<div class="ContentPasted0">>> the existing gem_create uAPI to let user set PAT index for the object</div>
<div class="ContentPasted0">>> at creation time.</div>
<div class="ContentPasted0">>> The new extension is platform independent, so UMD's can switch to using</div>
<div class="ContentPasted0">>> this extension for older platforms as well, while {set, get}_caching are</div>
<div class="ContentPasted0">>> still supported on these legacy paltforms for compatibility reason.</div>
<div class="ContentPasted0">>></div>
<div class="ContentPasted0">>> Test igt@gem_create@create_ext_set_pat posted at</div>
<div class="ContentPasted0">>> https://patchwork.freedesktop.org/series/117695/</div>
<div class="ContentPasted0">>></div>
<div class="ContentPasted0">>> Tested with https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22878</div>
<div class="ContentPasted0">>></div>
<div class="ContentPasted0">>> Signed-off-by: Fei Yang <fei.yang@intel.com></div>
<div class="ContentPasted0">>> Cc: Chris Wilson <chris.p.wilson@linux.intel.com></div>
<div class="ContentPasted0">>> Cc: Matt Roper <matthew.d.roper@intel.com></div>
<div class="ContentPasted0">>> Cc: Andi Shyti <andi.shyti@linux.intel.com></div>
<div class="ContentPasted0">>> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com></div>
<div class="ContentPasted0">>> Tested-by: Jordan Justen <jordan.l.justen@intel.com></div>
<div class="ContentPasted0">>> ---</div>
<div class="ContentPasted0">>> drivers/gpu/drm/i915/gem/i915_gem_create.c | 36 +++++++++++++++++++</div>
<div class="ContentPasted0">>> drivers/gpu/drm/i915/gem/i915_gem_object.c | 6 ++++</div>
<div class="ContentPasted0">>> include/uapi/drm/i915_drm.h | 42 ++++++++++++++++++++++</div>
<div class="ContentPasted0">>> tools/include/uapi/drm/i915_drm.h | 42 ++++++++++++++++++++++</div>
<div class="ContentPasted0">>> 4 files changed, 126 insertions(+)</div>
<div class="ContentPasted0">>></div>
<div class="ContentPasted0">>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c</div>
<div class="ContentPasted0">>> index bfe1dbda4cb7..644a936248ad 100644</div>
<div class="ContentPasted0">>> --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c</div>
<div class="ContentPasted0">>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c</div>
<div class="ContentPasted0">>> @@ -245,6 +245,7 @@ struct create_ext {</div>
<div class="ContentPasted0">>> unsigned int n_placements;</div>
<div class="ContentPasted0">>> unsigned int placement_mask;</div>
<div class="ContentPasted0">>> unsigned long flags;</div>
<div class="ContentPasted0">>> + unsigned int pat_index;</div>
<div class="ContentPasted0">>> };</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> static void repr_placements(char *buf, size_t size,</div>
<div class="ContentPasted0">>> @@ -394,11 +395,39 @@ static int ext_set_protected(struct i915_user_extension __user *base, void *data</div>
<div class="ContentPasted0">>> return 0;</div>
<div class="ContentPasted0">>> }</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> +static int ext_set_pat(struct i915_user_extension __user *base, void *data)</div>
<div class="ContentPasted0">>> +{</div>
<div class="ContentPasted0">>> + struct create_ext *ext_data = data;</div>
<div class="ContentPasted0">>> + struct drm_i915_private *i915 = ext_data->i915;</div>
<div class="ContentPasted0">>> + struct drm_i915_gem_create_ext_set_pat ext;</div>
<div class="ContentPasted0">>> + unsigned int max_pat_index;</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> + BUILD_BUG_ON(sizeof(struct drm_i915_gem_create_ext_set_pat) !=</div>
<div class="ContentPasted0">>> + offsetofend(struct drm_i915_gem_create_ext_set_pat, rsvd));</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> + if (copy_from_user(&ext, base, sizeof(ext)))</div>
<div class="ContentPasted0">>> + return -EFAULT;</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> + max_pat_index = INTEL_INFO(i915)->max_pat_index;</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> + if (ext.pat_index > max_pat_index) {</div>
<div class="ContentPasted0">>> + drm_dbg(&i915->drm, "PAT index is invalid: %u\n",</div>
<div class="ContentPasted0">>> + ext.pat_index);</div>
<div class="ContentPasted0">>> + return -EINVAL;</div>
<div class="ContentPasted0">>> + }</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> + ext_data->pat_index = ext.pat_index;</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> + return 0;</div>
<div class="ContentPasted0">>> +}</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> static const i915_user_extension_fn create_extensions[] = {</div>
<div class="ContentPasted0">>> [I915_GEM_CREATE_EXT_MEMORY_REGIONS] = ext_set_placements,</div>
<div class="ContentPasted0">>> [I915_GEM_CREATE_EXT_PROTECTED_CONTENT] = ext_set_protected,</div>
<div class="ContentPasted0">>> + [I915_GEM_CREATE_EXT_SET_PAT] = ext_set_pat,</div>
<div class="ContentPasted0">>> };</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> +#define PAT_INDEX_NOT_SET 0xffff</div>
<div class="ContentPasted0">>> /**</div>
<div class="ContentPasted0">>> * i915_gem_create_ext_ioctl - Creates a new mm object and returns a handle to it.</div>
<div class="ContentPasted0">>> * @dev: drm device pointer</div>
<div class="ContentPasted0">>> @@ -418,6 +447,7 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,</div>
<div class="ContentPasted0">>> if (args->flags & ~I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS)</div>
<div class="ContentPasted0">>> return -EINVAL;</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> + ext_data.pat_index = PAT_INDEX_NOT_SET;</div>
<div class="ContentPasted0">>> ret = i915_user_extensions(u64_to_user_ptr(args->extensions),</div>
<div class="ContentPasted0">>> create_extensions,</div>
<div class="ContentPasted0">>> ARRAY_SIZE(create_extensions),</div>
<div class="ContentPasted0">>> @@ -454,5 +484,11 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,</div>
<div class="ContentPasted0">>> if (IS_ERR(obj))</div>
<div class="ContentPasted0">>> return PTR_ERR(obj);</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> + if (ext_data.pat_index != PAT_INDEX_NOT_SET) {</div>
<div class="ContentPasted0">>> + i915_gem_object_set_pat_index(obj, ext_data.pat_index);</div>
<div class="ContentPasted0">>> + /* Mark pat_index is set by UMD */</div>
<div class="ContentPasted0">>> + obj->pat_set_by_user = true;</div>
<div class="ContentPasted0">>> + }</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> return i915_gem_publish(obj, file, &args->size, &args->handle);</div>
<div class="ContentPasted0">>> }</div>
<div class="ContentPasted0">>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c</div>
<div class="ContentPasted0">>> index 46a19b099ec8..97ac6fb37958 100644</div>
<div class="ContentPasted0">>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c</div>
<div class="ContentPasted0">>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c</div>
<div class="ContentPasted0">>> @@ -208,6 +208,12 @@ bool i915_gem_object_can_bypass_llc(struct drm_i915_gem_object *obj)</div>
<div class="ContentPasted0">>> if (!(obj->flags & I915_BO_ALLOC_USER))</div>
<div class="ContentPasted0">>> return false;</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> + /*</div>
<div class="ContentPasted0">>> + * Always flush cache for UMD objects at creation time.</div>
<div class="ContentPasted0">>> + */</div>
<div class="ContentPasted0">>> + if (obj->pat_set_by_user)</div>
<div class="ContentPasted0">>> + return true;</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> /*</div>
<div class="ContentPasted0">>> * EHL and JSL add the 'Bypass LLC' MOCS entry, which should make it</div>
<div class="ContentPasted0">>> * possible for userspace to bypass the GTT caching bits set by the</div>
<div class="ContentPasted0">>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h</div>
<div class="ContentPasted0">>> index ba40855dbc93..4f3fd650e5e1 100644</div>
<div class="ContentPasted0">>> --- a/include/uapi/drm/i915_drm.h</div>
<div class="ContentPasted0">>> +++ b/include/uapi/drm/i915_drm.h</div>
<div class="ContentPasted0">>> @@ -3664,9 +3664,13 @@ struct drm_i915_gem_create_ext {</div>
<div class="ContentPasted0">>> *</div>
<div class="ContentPasted0">>> * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see</div>
<div class="ContentPasted0">>> * struct drm_i915_gem_create_ext_protected_content.</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * For I915_GEM_CREATE_EXT_SET_PAT usage see</div>
<div class="ContentPasted0">>> + * struct drm_i915_gem_create_ext_set_pat.</div>
<div class="ContentPasted0">>> */</div>
<div class="ContentPasted0">>> #define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0</div>
<div class="ContentPasted0">>> #define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1</div>
<div class="ContentPasted0">>> +#define I915_GEM_CREATE_EXT_SET_PAT 2</div>
<div class="ContentPasted0">>> __u64 extensions;</div>
<div class="ContentPasted0">>> };</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> @@ -3781,6 +3785,44 @@ struct drm_i915_gem_create_ext_protected_content {</div>
<div class="ContentPasted0">>> __u32 flags;</div>
<div class="ContentPasted0">>> };</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> +/**</div>
<div class="ContentPasted0">>> + * struct drm_i915_gem_create_ext_set_pat - The</div>
<div class="ContentPasted0">>> + * I915_GEM_CREATE_EXT_SET_PAT extension.</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * If this extension is provided, the specified caching policy (PAT index) is</div>
<div class="ContentPasted0">>> + * applied to the buffer object.</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * Below is an example on how to create an object with specific caching policy:</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * .. code-block:: C</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * struct drm_i915_gem_create_ext_set_pat set_pat_ext = {</div>
<div class="ContentPasted0">>> + * .base = { .name = I915_GEM_CREATE_EXT_SET_PAT },</div>
<div class="ContentPasted0">>> + * .pat_index = 0,</div>
<div class="ContentPasted0">>> + * };</div>
<div class="ContentPasted0">>> + * struct drm_i915_gem_create_ext create_ext = {</div>
<div class="ContentPasted0">>> + * .size = PAGE_SIZE,</div>
<div class="ContentPasted0">>> + * .extensions = (uintptr_t)&set_pat_ext,</div>
<div class="ContentPasted0">>> + * };</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * int err = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext);</div>
<div class="ContentPasted0">>> + * if (err) ...</div>
<div class="ContentPasted0">>> + */</div>
<div class="ContentPasted0">>> +struct drm_i915_gem_create_ext_set_pat {</div>
<div class="ContentPasted0">>> + /** @base: Extension link. See struct i915_user_extension. */</div>
<div class="ContentPasted0">>> + struct i915_user_extension base;</div>
<div class="ContentPasted0">>> + /**</div>
<div class="ContentPasted0">>> + * @pat_index: PAT index to be set</div>
<div class="ContentPasted0">>> + * PAT index is a bit field in Page Table Entry to control caching</div>
<div class="ContentPasted0">>> + * behaviors for GPU accesses. The definition of PAT index is</div>
<div class="ContentPasted0">>> + * platform dependent and can be found in hardware specifications,</div>
<div class="ContentPasted0">>> + * e.g. BSpec 45101.</div>
<div class="ContentPasted0">></div>
<div class="ContentPasted0">> AFAIU BSpec numbers do not work from the outside so please put an url to</div>
<div class="ContentPasted0">> the place where PRMs are hosted instead.</div>
<div><br class="ContentPasted0">
</div>
<div class="ContentPasted0">Yeah, I was thinking of https://www.intel.com/content/www/us/en/docs/graphics-for-linux/developer-reference/1-0/hardware-specs.html</div>
<div class="ContentPasted0">But from these documents I could only find the definition of PTE which is</div>
<div class="ContentPasted0">referring the concept of to PAT: Page Attribute indirectly. There isn't any</div>
<div class="ContentPasted0">further detail and apparently the PAT tables are not included there either.</div>
<div><br class="ContentPasted0">
</div>
<div class="ContentPasted0">Any suggestion?</div>
<div><br class="ContentPasted0">
</div>
<div class="ContentPasted0">>> + */</div>
<div class="ContentPasted0">>> + __u32 pat_index;</div>
<div class="ContentPasted0">>> + /** @rsvd: reserved for future use */</div>
<div class="ContentPasted0">>> + __u32 rsvd;</div>
<div class="ContentPasted0">>> +};</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> /* ID of the protected content session managed by i915 when PXP is active */</div>
<div class="ContentPasted0">>> #define I915_PROTECTED_CONTENT_DEFAULT_SESSION 0xf</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> diff --git a/tools/include/uapi/drm/i915_drm.h b/tools/include/uapi/drm/i915_drm.h</div>
<div class="ContentPasted0">>> index 8df261c5ab9b..4fbfa472b9fc 100644</div>
<div class="ContentPasted0">>> --- a/tools/include/uapi/drm/i915_drm.h</div>
<div class="ContentPasted0">>> +++ b/tools/include/uapi/drm/i915_drm.h</div>
<div class="ContentPasted0">></div>
<div class="ContentPasted0">> Changes to this file I think you should drop. Looking at the log, it is</div>
<div class="ContentPasted0">> mostly other folks who synchronise it.</div>
<div><br class="ContentPasted0">
</div>
<div class="ContentPasted0">I don't really know how this is maintained, just grep'ed for i915 uAPIs and found</div>
<div class="ContentPasted0">this copy. But okay I can drop it if a different group of people is supposed to</div>
<div class="ContentPasted0">maintain it.</div>
<div><br class="ContentPasted0">
</div>
<div class="ContentPasted0">> Regards,</div>
<div class="ContentPasted0">></div>
<div class="ContentPasted0">> Tvrtko</div>
<div class="ContentPasted0">></div>
<div class="ContentPasted0">>> @@ -3607,9 +3607,13 @@ struct drm_i915_gem_create_ext {</div>
<div class="ContentPasted0">>> *</div>
<div class="ContentPasted0">>> * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see</div>
<div class="ContentPasted0">>> * struct drm_i915_gem_create_ext_protected_content.</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * For I915_GEM_CREATE_EXT_SET_PAT usage see</div>
<div class="ContentPasted0">>> + * struct drm_i915_gem_create_ext_set_pat.</div>
<div class="ContentPasted0">>> */</div>
<div class="ContentPasted0">>> #define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0</div>
<div class="ContentPasted0">>> #define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1</div>
<div class="ContentPasted0">>> +#define I915_GEM_CREATE_EXT_SET_PAT 2</div>
<div class="ContentPasted0">>> __u64 extensions;</div>
<div class="ContentPasted0">>> };</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> @@ -3724,6 +3728,44 @@ struct drm_i915_gem_create_ext_protected_content {</div>
<div class="ContentPasted0">>> __u32 flags;</div>
<div class="ContentPasted0">>> };</div>
<div class="ContentPasted0">>> </div>
<div class="ContentPasted0">>> +/**</div>
<div class="ContentPasted0">>> + * struct drm_i915_gem_create_ext_set_pat - The</div>
<div class="ContentPasted0">>> + * I915_GEM_CREATE_EXT_SET_PAT extension.</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * If this extension is provided, the specified caching policy (PAT index) is</div>
<div class="ContentPasted0">>> + * applied to the buffer object.</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * Below is an example on how to create an object with specific caching policy:</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * .. code-block:: C</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * struct drm_i915_gem_create_ext_set_pat set_pat_ext = {</div>
<div class="ContentPasted0">>> + * .base = { .name = I915_GEM_CREATE_EXT_SET_PAT },</div>
<div class="ContentPasted0">>> + * .pat_index = 0,</div>
<div class="ContentPasted0">>> + * };</div>
<div class="ContentPasted0">>> + * struct drm_i915_gem_create_ext create_ext = {</div>
<div class="ContentPasted0">>> + * .size = PAGE_SIZE,</div>
<div class="ContentPasted0">>> + * .extensions = (uintptr_t)&set_pat_ext,</div>
<div class="ContentPasted0">>> + * };</div>
<div class="ContentPasted0">>> + *</div>
<div class="ContentPasted0">>> + * int err = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext);</div>
<div class="ContentPasted0">>> + * if (err) ...</div>
<div class="ContentPasted0">>> + */</div>
<div class="ContentPasted0">>> +struct drm_i915_gem_create_ext_set_pat {</div>
<div class="ContentPasted0">>> + /** @base: Extension link. See struct i915_user_extension. */</div>
<div class="ContentPasted0">>> + struct i915_user_extension base;</div>
<div class="ContentPasted0">>> + /**</div>
<div class="ContentPasted0">>> + * @pat_index: PAT index to be set</div>
<div class="ContentPasted0">>> + * PAT index is a bit field in Page Table Entry to control caching</div>
<div class="ContentPasted0">>> + * behaviors for GPU accesses. The definition of PAT index is</div>
<div class="ContentPasted0">>> + * platform dependent and can be found in hardware specifications,</div>
<div class="ContentPasted0">>> + * e.g. BSpec 45101.</div>
<div class="ContentPasted0">>> + */</div>
<div class="ContentPasted0">>> + __u32 pat_index;</div>
<div class="ContentPasted0">>> + /** @rsvd: reserved for future use */</div>
<div class="ContentPasted0">>> + __u32 rsvd;</div>
<div class="ContentPasted0">>> +};</div>
<div class="ContentPasted0">>> +</div>
<div class="ContentPasted0">>> /* ID of the protected content session managed by i915 when PXP is active */</div>
<div class="ContentPasted0">>> #define I915_PROTECTED_CONTENT_DEFAULT_SESSION 0xf</div>
<div class="ContentPasted0">>> </div>
</div>
</body>
</html>