| Message ID | 20140625154533.1691900c@jbarnes-desktop (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 25, 2014 at 03:45:33PM -0700, Jesse Barnes wrote: > Apparently trinary logic is hard. We were falling through all the forced > cases and simply enabling aliasing PPGTT or not based on hardware, > rather than full PPGTT if available. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=80083 > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index a4153ee..86521a7 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -69,7 +69,13 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) > return 0; > } > > - return HAS_ALIASING_PPGTT(dev) ? 1 : 0; > + /* Fall through to auto-detect */ > + if (HAS_PPGTT(dev)) > + return 2; > + else if (HAS_ALIASING_PPGTT(dev)) > + return 1; > + > + return 0; I don't get it. This would just enable full ppgtt by default. But full ppgtt is still a bit broken so I don't think we want this. The dmesg in the bug shows that ppgtt was forced off by the PCI revision check, so that part seems to have worked. Looks like the tests doesn't expect the batches to be accepted by the kernel, but the cmd parser is bypassed when ppgtt is disabled, so the test fails.
On Thu, 26 Jun 2014 17:59:48 +0300 Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Jun 25, 2014 at 03:45:33PM -0700, Jesse Barnes wrote: > > Apparently trinary logic is hard. We were falling through all the forced > > cases and simply enabling aliasing PPGTT or not based on hardware, > > rather than full PPGTT if available. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=80083 > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index a4153ee..86521a7 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -69,7 +69,13 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) > > return 0; > > } > > > > - return HAS_ALIASING_PPGTT(dev) ? 1 : 0; > > + /* Fall through to auto-detect */ > > + if (HAS_PPGTT(dev)) > > + return 2; > > + else if (HAS_ALIASING_PPGTT(dev)) > > + return 1; > > + > > + return 0; > > I don't get it. This would just enable full ppgtt by default. But full > ppgtt is still a bit broken so I don't think we want this. > > The dmesg in the bug shows that ppgtt was forced off by the PCI revision > check, so that part seems to have worked. Looks like the tests doesn't > expect the batches to be accepted by the kernel, but the cmd parser is > bypassed when ppgtt is disabled, so the test fails. Yeah and I was testing on a platform that had PPGTT support, and figured I had screwed up the PPGTT enable. But I guess not, and we can expect these tests to fail until we have the command parser enabled everywhere.
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index a4153ee..86521a7 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -69,7 +69,13 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) return 0; } - return HAS_ALIASING_PPGTT(dev) ? 1 : 0; + /* Fall through to auto-detect */ + if (HAS_PPGTT(dev)) + return 2; + else if (HAS_ALIASING_PPGTT(dev)) + return 1; + + return 0; }
Apparently trinary logic is hard. We were falling through all the forced cases and simply enabling aliasing PPGTT or not based on hardware, rather than full PPGTT if available. References: https://bugs.freedesktop.org/show_bug.cgi?id=80083 Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)