| Message ID | 1401314544-3247-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 28, 2014 at 03:02:24PM -0700, Jesse Barnes wrote: > Need testing and possibly disabling on earlier steppings, but looks ok > here on my B3. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index ec5f6fb..4b0e58c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1971,7 +1971,7 @@ struct drm_i915_cmd_table { > > #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) > #define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev)) > -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) \ > +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \ > && !IS_BROADWELL(dev)) Which branch is this against? In -nightly, these are implemented differently, so I don't know if all comments will apply. But... Since IS_VALLEYVIEW() is also true for chv, should we make the condition !IS_CHERRYVIEW() in this patch instead? Or do we intend to have PPGTT enabled for chv? I think we need the corresponding change to HAS_ALIASING_PPGTT() in order to actually enable PPGTT. Otherwise it looks like sanitize_enable_ppgtt() will disable PPGTT. Brad > #define USES_PPGTT(dev) intel_enable_ppgtt(dev, false) > #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true) > -- > 1.8.4.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 29 May 2014 08:30:10 -0700 "Volkin, Bradley D" <bradley.d.volkin@intel.com> wrote: > On Wed, May 28, 2014 at 03:02:24PM -0700, Jesse Barnes wrote: > > Need testing and possibly disabling on earlier steppings, but looks ok > > here on my B3. > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index ec5f6fb..4b0e58c 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1971,7 +1971,7 @@ struct drm_i915_cmd_table { > > > > #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) > > #define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev)) > > -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) \ > > +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \ > > && !IS_BROADWELL(dev)) > > Which branch is this against? In -nightly, these are implemented > differently, so I don't know if all comments will apply. But... > > Since IS_VALLEYVIEW() is also true for chv, should we make the condition > !IS_CHERRYVIEW() in this patch instead? Or do we intend to have PPGTT > enabled for chv? > > I think we need the corresponding change to HAS_ALIASING_PPGTT() in order > to actually enable PPGTT. Otherwise it looks like sanitize_enable_ppgtt() > will disable PPGTT. I knew this was too easy, maybe that's why it worked so well for Ville and I... unless I was somehow testing full PPGTT. I'll check it out.
On Thu, May 29, 2014 at 08:44:51AM -0700, Jesse Barnes wrote: > On Thu, 29 May 2014 08:30:10 -0700 > "Volkin, Bradley D" <bradley.d.volkin@intel.com> wrote: > > > On Wed, May 28, 2014 at 03:02:24PM -0700, Jesse Barnes wrote: > > > Need testing and possibly disabling on earlier steppings, but looks ok > > > here on my B3. > > > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > index ec5f6fb..4b0e58c 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -1971,7 +1971,7 @@ struct drm_i915_cmd_table { > > > > > > #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) > > > #define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev)) > > > -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) \ > > > +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \ > > > && !IS_BROADWELL(dev)) > > > > Which branch is this against? In -nightly, these are implemented > > differently, so I don't know if all comments will apply. But... > > > > Since IS_VALLEYVIEW() is also true for chv, should we make the condition > > !IS_CHERRYVIEW() in this patch instead? Or do we intend to have PPGTT > > enabled for chv? > > > > I think we need the corresponding change to HAS_ALIASING_PPGTT() in order > > to actually enable PPGTT. Otherwise it looks like sanitize_enable_ppgtt() > > will disable PPGTT. > > I knew this was too easy, maybe that's why it worked so well for Ville > and I... unless I was somehow testing full PPGTT. I'll check it out. #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) -#define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev)) -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) \ +#define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6) +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \ && !IS_BROADWELL(dev)) is what I see in my branch, so I'm pretty sure I actually tested aliasing ppgtt. And IIRC I tried full ppgtt too (such as it was ~2 months ago).
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ec5f6fb..4b0e58c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1971,7 +1971,7 @@ struct drm_i915_cmd_table { #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) #define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev)) -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) \ +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \ && !IS_BROADWELL(dev)) #define USES_PPGTT(dev) intel_enable_ppgtt(dev, false) #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true)
Need testing and possibly disabling on earlier steppings, but looks ok here on my B3. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_drv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)