On Fri, 25 May 2012 10:39:57 -0700
Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> On Fri, 27 Apr 2012 17:40:20 -0700
> Ben Widawsky <ben at bwidawsk.net> wrote:
>
> > If any l3 rows have been previously remapped, we must remap them after
> > GPU reset/resume too.
> >
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 3 +++
> > drivers/gpu/drm/i915/i915_gem.c | 26 ++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > 3 files changed, 32 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 9505fc0..e9efe17 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -641,6 +641,8 @@ typedef struct drm_i915_private {
> > /** PPGTT used for aliasing the PPGTT with the GTT */
> > struct i915_hw_ppgtt *aliasing_ppgtt;
> >
> > + u32 *l3_remap_info;
> > +
> > struct shrinker inactive_shrinker;
> >
> > /**
> > @@ -1290,6 +1292,7 @@ int __must_check i915_gem_object_set_domain(struct drm_i915_gem_object *obj,
> > uint32_t write_domain);
> > int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
> > int __must_check i915_gem_init_hw(struct drm_device *dev);
> > +void i915_gem_l3_remap(struct drm_device *dev);
> > void i915_gem_init_swizzling(struct drm_device *dev);
> > void i915_gem_init_ppgtt(struct drm_device *dev);
> > void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 7bc4a40..bb3ef9f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3475,6 +3475,30 @@ i915_gem_idle(struct drm_device *dev)
> > return 0;
> > }
> >
> > +void i915_gem_l3_remap(struct drm_device *dev)
> > +{
> > + drm_i915_private_t *dev_priv = dev->dev_private;
> > + u32 misccpctl;
> > + int i;
> > +
> > + if (!dev_priv->mm.l3_remap_info)
> > + return;
> > +
> > + WARN_ON(!IS_IVYBRIDGE(dev));
>
> Should just be a return?
I had this here since it's a public function, and we have a history of
calling functions on the wrong generation. So it just helps us find out
a little sooner we're doing the wrong thing. As you point out below
though, this doesn't give the behavior we want. Thanks for catching it.
>
> > +
> > + misccpctl = I915_READ(GEN7_MISCCPCTL);
> > + I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE);
> > + POSTING_READ(GEN7_MISCCPCTL);
> > +
> > + for (i = 0; i < GEN7_L3LOG_SIZE; i += 4)
> > + I915_WRITE(GEN7_L3LOG_BASE + i, dev_priv->mm.l3_remap_info[i/4]);
> > +
> > + /* Make sure all the writes land before disabling dop clock gating */
> > + POSTING_READ(GEN7_L3LOG_BASE);
>
> Same comment as before on the DOP gating...
I think we're good here now too.
>
> > +
> > + I915_WRITE(GEN7_MISCCPCTL, misccpctl);
> > +}
> > +
> > void i915_gem_init_swizzling(struct drm_device *dev)
> > {
> > drm_i915_private_t *dev_priv = dev->dev_private;
> > @@ -3566,6 +3590,8 @@ i915_gem_init_hw(struct drm_device *dev)
> > drm_i915_private_t *dev_priv = dev->dev_private;
> > int ret;
> >
> > + i915_gem_l3_remap(dev);
>
> Since this looks unconditional, so we'll get the backtrace on every
> non-IVB system?
Yes. That seems like a problem.
>
>
>
Reviewed-by?
--
Ben Widawsky, Intel Open Source Technology Center
