| Message ID | 20221004114915.221708-2-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/2] drm/i915: enable PS64 support for DG2 | expand |
On 10/4/2022 1:49 PM, 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> Reviewed-by: 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
Acked-by: Jordan Justen <jordan.l.justen@intel.com> 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 >
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 >>
On 2022-10-14 03:58:12, Matthew Auld wrote: > 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. Speaking from the uapi perspective, I would would say, yes, it looks seems good to merge. I looked over your igt patches, and they seem to test the uapi well. The uapi change is pretty simple, and the related documentation changes look good. No, we haven't gotten the chance to test the uapi implementation with Mesa, but I expect we will soon, once it's in drm-tip. -Jordan
On 14/10/2022 17:51, Jordan Justen wrote: > On 2022-10-14 03:58:12, Matthew Auld wrote: >> 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. > > Speaking from the uapi perspective, I would would say, yes, it looks > seems good to merge. I looked over your igt patches, and they seem to > test the uapi well. The uapi change is pretty simple, and the related > documentation changes look good. > > No, we haven't gotten the chance to test the uapi implementation with > Mesa, but I expect we will soon, once it's in drm-tip. Pushed to drm-tip. Thanks. > > -Jordan
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
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(-)