| Message ID | 1366316916-4164-3-git-send-email-kenneth@whitecape.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 18 Apr 2013, Kenneth Graunke <kenneth@whitecape.org> wrote: > On Bay Trail, bit 1 means "writeable by the GPU." Failing to set that > means basically anything using the GPU will cause hangs. > > Signed-off-by: Kenneth Graunke <kenneth@whitecape.org> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 33 +++++++++++++++++++++++++++++++-- > 1 file changed, 31 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 9db65c1..db5c654 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -75,6 +75,27 @@ static inline gen6_gtt_pte_t gen6_pte_encode(struct drm_device *dev, > return pte; > } > > +#define BYT_PTE_WRITEABLE (1 << 1) > +#define BYT_PTE_SNOOPED_BY_CPU_CACHES (1 << 2) > + > +static inline gen6_gtt_pte_t byt_pte_encode(struct drm_device *dev, inline doesn't really make sense for functions used through pointers... I wonder if the compiler should warn about taking address of inline functions, or whether it just happily non-inlines. Too lazy to check. ;) Oh, this applies for patch 1 too; please drop inline from gen6_pte_encode() there. > + dma_addr_t addr, > + enum i915_cache_level level) > +{ > + gen6_gtt_pte_t pte = GEN6_PTE_VALID; > + pte |= GEN6_PTE_ADDR_ENCODE(addr); > + > + /* Mark the page as writeable. Other platforms don't have a > + * setting for read-only/writable, so this matches that behavior. > + */ > + pte |= BYT_PTE_WRITEABLE; > + > + if (level != I915_CACHE_NONE) > + pte |= BYT_PTE_SNOOPED_BY_CPU_CACHES; > + > + return pte; > +} > + > static int gen6_ppgtt_enable(struct drm_device *dev) > { > drm_i915_private_t *dev_priv = dev->dev_private; > @@ -236,7 +257,11 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) > first_pd_entry_in_global_pt = > gtt_total_entries(dev_priv->gtt) - I915_PPGTT_PD_ENTRIES; > > - ppgtt->pte_encode = gen6_pte_encode; > + if (IS_VALLEYVIEW(dev)) { > + ppgtt->pte_encode = byt_pte_encode; > + } else { > + ppgtt->pte_encode = gen6_pte_encode; > + } HAS_ALIASING_PPGTT(dev) returns false for VLV. Do we need this in preparation for future, or can we just drop this? > ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES; > ppgtt->enable = gen6_ppgtt_enable; > ppgtt->clear_range = gen6_ppgtt_clear_range; > @@ -826,7 +851,11 @@ int i915_gem_gtt_init(struct drm_device *dev) > } else { > dev_priv->gtt.gtt_probe = gen6_gmch_probe; > dev_priv->gtt.gtt_remove = gen6_gmch_remove; > - dev_priv->gtt.pte_encode = gen6_pte_encode; > + if (IS_VALLEYVIEW(dev)) { > + dev_priv->gtt.pte_encode = byt_pte_encode; > + } else { > + dev_priv->gtt.pte_encode = gen6_pte_encode; > + } The rest looks good. With the bikesheds addressed (by patches or counter-arguments ;) Reviewed-by: Jani Nikula <jani.nikula@intel.com> > } > > ret = dev_priv->gtt.gtt_probe(dev, &dev_priv->gtt.total, > -- > 1.8.2.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 9db65c1..db5c654 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -75,6 +75,27 @@ static inline gen6_gtt_pte_t gen6_pte_encode(struct drm_device *dev, return pte; } +#define BYT_PTE_WRITEABLE (1 << 1) +#define BYT_PTE_SNOOPED_BY_CPU_CACHES (1 << 2) + +static inline gen6_gtt_pte_t byt_pte_encode(struct drm_device *dev, + dma_addr_t addr, + enum i915_cache_level level) +{ + gen6_gtt_pte_t pte = GEN6_PTE_VALID; + pte |= GEN6_PTE_ADDR_ENCODE(addr); + + /* Mark the page as writeable. Other platforms don't have a + * setting for read-only/writable, so this matches that behavior. + */ + pte |= BYT_PTE_WRITEABLE; + + if (level != I915_CACHE_NONE) + pte |= BYT_PTE_SNOOPED_BY_CPU_CACHES; + + return pte; +} + static int gen6_ppgtt_enable(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; @@ -236,7 +257,11 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) first_pd_entry_in_global_pt = gtt_total_entries(dev_priv->gtt) - I915_PPGTT_PD_ENTRIES; - ppgtt->pte_encode = gen6_pte_encode; + if (IS_VALLEYVIEW(dev)) { + ppgtt->pte_encode = byt_pte_encode; + } else { + ppgtt->pte_encode = gen6_pte_encode; + } ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES; ppgtt->enable = gen6_ppgtt_enable; ppgtt->clear_range = gen6_ppgtt_clear_range; @@ -826,7 +851,11 @@ int i915_gem_gtt_init(struct drm_device *dev) } else { dev_priv->gtt.gtt_probe = gen6_gmch_probe; dev_priv->gtt.gtt_remove = gen6_gmch_remove; - dev_priv->gtt.pte_encode = gen6_pte_encode; + if (IS_VALLEYVIEW(dev)) { + dev_priv->gtt.pte_encode = byt_pte_encode; + } else { + dev_priv->gtt.pte_encode = gen6_pte_encode; + } } ret = dev_priv->gtt.gtt_probe(dev, &dev_priv->gtt.total,
On Bay Trail, bit 1 means "writeable by the GPU." Failing to set that means basically anything using the GPU will cause hangs. Signed-off-by: Kenneth Graunke <kenneth@whitecape.org> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-)