Re: [Intel-gfx] [PATCH v5 1/5] drm/i915: add needs_compact_pt flag

[Thomas Hellström (Intel)]

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@xxxxxxxxx>
> > >
> > > 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@xxxxxxxxx>
> > > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> > > Signed-off-by: Robert Beckett <bob.beckett@xxxxxxxxxxxxx>
> > > ---
> > >   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.

/Thomas