Messages in this thread | | | Date | Mon, 31 Jan 2022 15:58:26 +0100 | Subject | Re: [Intel-gfx] [PATCH v5 1/5] drm/i915: add needs_compact_pt flag | From | Thomas Hellström <> |
| |
On 1/31/22 15:19, Robert Beckett wrote: > > > On 27/01/2022 09:37, Thomas Hellström (Intel) wrote: >> >> On 1/26/22 18:11, Robert Beckett wrote: >>> >>> >>> On 26/01/2022 13:49, Thomas Hellström (Intel) wrote: >>>> >>>> On 1/25/22 20:35, Robert Beckett wrote: >>>>> From: Ramalingam C <ramalingam.c@intel.com> >>>>> >>>>> Add a new platform flag, needs_compact_pt, to mark the requirement of >>>>> compact pt layout support for the ppGTT when using 64K GTT pages. >>>>> >>>>> With this flag has_64k_pages will only indicate requirement of 64K >>>>> GTT page sizes or larger for device local memory access. >>>>> >>>>> Suggested-by: Matthew Auld <matthew.auld@intel.com> >>>>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> >>>>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com> >>>>> --- >>>>> drivers/gpu/drm/i915/i915_drv.h | 10 +++++++--- >>>>> drivers/gpu/drm/i915/i915_pci.c | 2 ++ >>>>> drivers/gpu/drm/i915/intel_device_info.h | 1 + >>>>> 3 files changed, 10 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h >>>>> b/drivers/gpu/drm/i915/i915_drv.h >>>>> index 44c1f98144b4..1258b7779705 100644 >>>>> --- a/drivers/gpu/drm/i915/i915_drv.h >>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>>>> @@ -1512,12 +1512,16 @@ IS_SUBPLATFORM(const struct >>>>> drm_i915_private *i915, >>>>> /* >>>>> * Set this flag, when platform requires 64K GTT page sizes or >>>>> larger for >>>>> - * device local memory access. Also this flag implies that we >>>>> require or >>>>> - * at least support the compact PT layout for the ppGTT when >>>>> using the 64K >>>>> - * GTT pages. >>>> >>>> Why do we remove these comment lines? >>> Because HAS_64K_PAGES now means just 64K page, it no longer means >>> also requires compact pt. >>> This is to support other products that will have 64K but not have >>> the PDE non-sharing restriction in future. >>> >>> Those lines moved to the next change NEEDS_COMPACT_PT, which is now >>> separate. >> >> Yes, NEEDS_COMPACT_PT indicates that compact is *required* but does >> "HAS_64K_PAGES" still mean compact is supported? That information is >> lost. > Not any more. > I discussed the ambiguity of the original wording with mauld on irc. > We came to the conclusion that HAS_64K_PAGES should mean just that, > that the hw has support for 64K pages, and says nothing about > compact-pt at all. > > NEEDS_COMPACT_PT means that the hw has compact-pt support and the > driver is required to use it as it is a hw limitation. > > There will be other devices that can support compact-pt but do not > mandate its use. In this case, the current code would not use them, > but there is potential for some future opportunistic use of that in > the driver if desired (currently expected to include accelerated > move/clear). If any opportunistic use is added to the driver, a new > flag can be added along with the code that uses it to indicate > compact-pt availability that is not mandatory (HAS_COMPACT_PT most > likely), but as there is no code requiring it currently it should not > be added yet, and the comments left as this patch does. > Ok, Thanks for the clarification.
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> >> /Thomas >> >>
| |