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

[Bob Paauwe <bob.j.paauwe@intel.com>]

On Thu, 13 Sep 2018 20:22:14 +0300
Ville Syrjälä <ville.syrjala@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@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@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.  If it's important to actually test
larger values, that could be a separate change to the mock device.

Bob

--
Bob Paauwe                  
Bob.J.Paauwe@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193    

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx