[Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v4)

[Chris Wilson]

Quoting Bob Paauwe (2018-09-14 16:51:34)
> On Thu, 13 Sep 2018 20:22:14 +0300
> Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> 
> > On Thu, Sep 13, 2018 at 10:12:06AM -0700, Bob Paauwe wrote:
> > > On Thu, 13 Sep 2018 20:05:54 +0300
> > > Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> > >   
> > > > On Thu, Sep 13, 2018 at 10:02:57AM -0700, Bob Paauwe wrote:  
> > > > > On Wed, 12 Sep 2018 17:10:58 +0100
> > > > > Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > > > >     
> > > > > > Quoting Bob Paauwe (2018-09-12 17:04:30)    
> > > > > > > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > > > > > index 43ed8b28aeaa..33d7225edbbb 100644
> > > > > > > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > > > > > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > > > > > @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void)
> > > > > > >                 I915_GTT_PAGE_SIZE_64K |
> > > > > > >                 I915_GTT_PAGE_SIZE_2M;
> > > > > > >  
> > > > > > > +       mkwrite_device_info(i915)->full_ppgtt_bits = 48;      
> > > > > > 
> > > > > > Actually the mock ppgtt is 64b.
> > > > > > -Chris    
> > > > > 
> > > > > Setting it 64 bit causes mock_hugepages to fail.    
> > > > 
> > > > 1<<64 somewhere?
> > > >   
> > > 
> > > Doh, yeah, there is that. So what does 64b mean for ppgtt->vm.total?
> > > Should it really be 63b?  
> > 
> > GENMASK() & ~(GTT_PAGE_SIZE-1) maybe?
> 
> This seems to be getting somewhat outside the scope of this patch, but
> I did some experimenting and tried to get a better understanding of
> how the ppgtt-vm.total value is used.
> 
> The hugepages selftest will work if this is set to 1 << 63 for the mock
> device. It also seems to work if I do something like what Ville
> suggests and set it to ((1 << 64) - 1) & ~(I915_GTT_PAGE_SIZE - 1)
> which, if I don't typo it, is 0xfffffffffffff000.
> 
> So I could change the way ppgtt->vm.total is set to something like:
> 
> GENMASK_ULL(bits - 1, 0) & ~(I915_GTT_PAGE_SIZE - 1)
> 
> and it would set it to a slightly smaller value than what we do now, but
> even if that passes the regression tests, it is a change in behavior
> and I don't think that should be part of this patch.
> 
> My preference would be to leave the mock device configured to 48 bits
> since that what it has been using.

The mock device was intentionally meant to be testing the full range
that the driver supports, not the hw. (That it has to be limited to fit
in memory is another matter for some tests.)
-Chris