Re: [PATCH] drm/i915: Disable caches for Global GTT.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Oct 31, 2014 at 07:44:34AM +0000, Chris Wilson wrote:
> On Thu, Oct 30, 2014 at 09:18:18AM -0700, Rodrigo Vivi wrote:
> > Global GTT doesn't have pat_sel[2:0] so it always point to pat_sel = 000;
> > So the only way to avoid screen corruptions is setting PAT 0 to Uncached.
> > 
> > MOCS can still be used though. But if userspace is trusting PTE for
> > cache selection the safest thing to do is to let caches disabled.
> > 
> > BSpec: "For GGTT, there is NO pat_sel[2:0] from the entry,
> > so RTL will always use the value corresponding to pat_sel = 000"
> > 
> > Reference: https://bugs.freedesktop.org/show_bug.cgi?id=85576
> > Cc: James Ausmus <james.ausmus@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 29 +++++++++++++++++++++--------
> >  1 file changed, 21 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index cb7adab..24b4f27 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -1911,14 +1911,27 @@ static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv)
> >  {
> >  	uint64_t pat;
> >  
> > -	pat = GEN8_PPAT(0, GEN8_PPAT_WB | GEN8_PPAT_LLC)     | /* for normal objects, no eLLC */
> > -	      GEN8_PPAT(1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC) | /* for something pointing to ptes? */
> > -	      GEN8_PPAT(2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC) | /* for scanout with eLLC */
> > -	      GEN8_PPAT(3, GEN8_PPAT_UC)                     | /* Uncached objects, mostly for scanout */
> > -	      GEN8_PPAT(4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0)) |
> > -	      GEN8_PPAT(5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1)) |
> > -	      GEN8_PPAT(6, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(2)) |
> > -	      GEN8_PPAT(7, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3));
> 
> Instead of deleting this and making a rather ugly if,
> 
> Just
> > +	if (!USES_PPGTT(dev_priv))
> > +		/* Spec: "For GGTT, there is NO pat_sel[2:0] from the entry,
> > +		 * so RTL will always use the value corresponding to
> > +		 * pat_sel = 000".
> > +		 * So let's disable cache for GGTT to avoid screen corruptions.
> > +		 * MOCS still can be used though.
> > +		 */
> > +		pat = GEN8_PPAT(0, GEN8_PPAT_UC);
> 
> to override in the unlikely case of disabling ppgtt.

Well GGTT is broken whether or not PPGTT is enabled, so I belive we should
just swap two of the entires around WB<->UC, and then update the index
defines to match, and CHV needs the same change as well.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx