Re: [Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v4)
Quoting Bob Paauwe (2018-09-14 16:51:34) > On Thu, 13 Sep 2018 20:22:14 +0300 > Ville Syrjälä 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ä 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 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 0xf000. > > 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v4)
On Thu, 13 Sep 2018 20:22:14 +0300 Ville Syrjälä 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ä 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 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 0xf000. 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.paa...@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
Re: [Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v4)
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ä 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 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? -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v4)
On Thu, 13 Sep 2018 20:05:54 +0300 Ville Syrjälä 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 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? -- Bob Paauwe bob.j.paa...@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
Re: [Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v4)
On Thu, Sep 13, 2018 at 10:02:57AM -0700, Bob Paauwe wrote: > On Wed, 12 Sep 2018 17:10:58 +0100 > Chris Wilson 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? -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v4)
On Wed, 12 Sep 2018 17:10:58 +0100 Chris Wilson 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. I don't believe the current driver code ever uses anything other than 32 or 48 bit so this may mean there's a bug somewhere if we go over 48 bit. Bob -- Bob Paauwe bob.j.paa...@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
Re: [Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v4)
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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
