Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/uapi: expose GTT alignment

[Matthew Auld <matthew.auld@intel.com>]

On 14/10/2022 08:20, Jordan Justen wrote:
> Acked-by: Jordan Justen <jordan.l.justen@intel.com>

Thanks. Can I take that as ack for merging the series from Mesa POV? I 
think Lionel was going to test this, but I think keeps getting swamped 
with other stuff. We kind of urgently need to land this series.

> 
> On 2022-10-04 04:49:15, Matthew Auld wrote:
>> On some platforms we potentially have different alignment restrictions
>> depending on the memory type. We also now have different alignment
>> restrictions for the same region across different kernel versions.
>> Extend the region query to return the minimum required GTT alignment.
>>
>> Testcase: igt@gem_create@create-ext-placement-alignment
>> Testcase: igt@i915_query@query-regions-sanity-check
>> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Michal Mrozek <michal.mrozek@intel.com>
>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Cc: Stuart Summers <stuart.summers@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Yang A Shi <yang.a.shi@intel.com>
>> Cc: Nirmoy Das <nirmoy.das@intel.com>
>> Cc: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_query.c |  1 +
>>   include/uapi/drm/i915_drm.h       | 29 +++++++++++++++++++++++++++--
>>   2 files changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
>> index 6ec9c9fb7b0d..111377f210ed 100644
>> --- a/drivers/gpu/drm/i915/i915_query.c
>> +++ b/drivers/gpu/drm/i915/i915_query.c
>> @@ -498,6 +498,7 @@ static int query_memregion_info(struct drm_i915_private *i915,
>>                  info.region.memory_class = mr->type;
>>                  info.region.memory_instance = mr->instance;
>>                  info.probed_size = mr->total;
>> +               info.gtt_alignment = mr->min_page_size;
>>   
>>                  if (mr->type == INTEL_MEMORY_LOCAL)
>>                          info.probed_cpu_visible_size = mr->io_size;
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index 08d69e36fb66..2e613109356b 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -3346,8 +3346,33 @@ struct drm_i915_memory_region_info {
>>          /** @region: The class:instance pair encoding */
>>          struct drm_i915_gem_memory_class_instance region;
>>   
>> -       /** @rsvd0: MBZ */
>> -       __u32 rsvd0;
>> +       union {
>> +               /** @rsvd0: MBZ */
>> +               __u32 rsvd0;
>> +               /**
>> +                * @gtt_alignment:
>> +                *
>> +                * The minimum required GTT alignment for this type of memory.
>> +                * When allocating a GTT address it must be aligned to this
>> +                * value or larger. On some platforms the kernel might opt to
>> +                * using 64K pages for I915_MEMORY_CLASS_DEVICE, where 64K GTT
>> +                * pages can then be used if we also use 64K GTT alignment.
>> +                *
>> +                * NOTE: If this is zero then this must be an older
>> +                * kernel which lacks support for this field.
>> +                *
>> +                * Side note: For larger objects (especially for
>> +                * I915_MEMORY_CLASS_DEVICE), like 2M+ in size, userspace should
>> +                * consider potentially bumping the GTT alignment to say 2M,
>> +                * which could potentially increase the likelihood of the kernel
>> +                * being able to utilise 2M GTT pages underneath, if the layout
>> +                * of the physical pages allows it.  On some configurations we
>> +                * can then also use a more efficient page-table layout, if we
>> +                * can't use the more desirable 2M GTT page, so long as we know
>> +                * that the entire page-table will be used by this object.
>> +                */
>> +               __u32 gtt_alignment;
>> +       };
>>   
>>          /**
>>           * @probed_size: Memory probed by the driver
>> -- 
>> 2.37.3
>>