From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 3/6] drm/i915: Make l3 remapping use the ring Date: Thu, 19 Sep 2013 20:39:18 +0200 Message-ID: <20130919183918.GA32145@phenom.ffwll.local> References: <1379477575-2164-1-git-send-email-benjamin.widawsky@intel.com> <1379477575-2164-3-git-send-email-benjamin.widawsky@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f43.google.com (mail-ee0-f43.google.com [74.125.83.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 56BE9E5C85 for ; Thu, 19 Sep 2013 11:39:03 -0700 (PDT) Received: by mail-ee0-f43.google.com with SMTP id e52so4339676eek.2 for ; Thu, 19 Sep 2013 11:39:02 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1379477575-2164-3-git-send-email-benjamin.widawsky@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky Cc: Intel GFX , Bryan Bell , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Tue, Sep 17, 2013 at 09:12:44PM -0700, Ben Widawsky wrote: > Using LRI for setting the remapping registers allows us to stream l3 > remapping information. This is necessary to handle per context remaps as > we'll see implemented in an upcoming patch. > > Using the ring also means we don't need to frob the DOP clock gating > bits. > > v2: Add comment about lack of worry for concurrent register access > (Daniel) > > Signed-off-by: Ben Widawsky I've done a s/XXX/Note on the comment and converted it into the preferred layout for multiline comments. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/i915_gem.c | 38 ++++++++++++++++++-------------------- > drivers/gpu/drm/i915/i915_sysfs.c | 3 ++- > 3 files changed, 21 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index c6e8df7..0c39805 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1949,7 +1949,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force); > int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj); > int __must_check i915_gem_init(struct drm_device *dev); > int __must_check i915_gem_init_hw(struct drm_device *dev); > -void i915_gem_l3_remap(struct drm_device *dev, int slice); > +int i915_gem_l3_remap(struct intel_ring_buffer *ring, int slice); > void i915_gem_init_swizzling(struct drm_device *dev); > void i915_gem_cleanup_ringbuffer(struct drm_device *dev); > int __must_check i915_gpu_idle(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 66bf75d..2538138 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4252,35 +4252,33 @@ i915_gem_idle(struct drm_device *dev) > return 0; > } > > -void i915_gem_l3_remap(struct drm_device *dev, int slice) > +int i915_gem_l3_remap(struct intel_ring_buffer *ring, int slice) > { > + struct drm_device *dev = ring->dev; > drm_i915_private_t *dev_priv = dev->dev_private; > u32 reg_base = GEN7_L3LOG_BASE + (slice * 0x200); > u32 *remap_info = dev_priv->l3_parity.remap_info[slice]; > - u32 misccpctl; > - int i; > + int i, ret; > > if (!HAS_L3_GPU_CACHE(dev) || !remap_info) > - return; > + return 0; > > - misccpctl = I915_READ(GEN7_MISCCPCTL); > - I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE); > - POSTING_READ(GEN7_MISCCPCTL); > + ret = intel_ring_begin(ring, GEN7_L3LOG_SIZE / 4 * 3); > + if (ret) > + return ret; > > + /* XXX: We do not worry about the concurrent register cacheline hang > + * here because no other code should access these registers other than > + * at initialization time. */ > for (i = 0; i < GEN7_L3LOG_SIZE; i += 4) { > - u32 remap = I915_READ(reg_base + i); > - if (remap && remap != remap_info[i/4]) > - DRM_DEBUG("0x%x was already programmed to %x\n", > - reg_base + i, remap); > - if (remap && !remap_info[i/4]) > - DRM_DEBUG_DRIVER("Clearing remapped register\n"); > - I915_WRITE(reg_base + i, remap_info[i/4]); > + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); > + intel_ring_emit(ring, reg_base + i); > + intel_ring_emit(ring, remap_info[i/4]); > } > > - /* Make sure all the writes land before disabling dop clock gating */ > - POSTING_READ(reg_base); > + intel_ring_advance(ring); > > - I915_WRITE(GEN7_MISCCPCTL, misccpctl); > + return ret; > } > > void i915_gem_init_swizzling(struct drm_device *dev) > @@ -4391,15 +4389,15 @@ i915_gem_init_hw(struct drm_device *dev) > I915_WRITE(GEN7_MSG_CTL, temp); > } > > - for (i = 0; i < NUM_L3_SLICES(dev); i++) > - i915_gem_l3_remap(dev, i); > - > i915_gem_init_swizzling(dev); > > ret = i915_gem_init_rings(dev); > if (ret) > return ret; > > + for (i = 0; i < NUM_L3_SLICES(dev); i++) > + i915_gem_l3_remap(&dev_priv->ring[RCS], i); > + > /* > * XXX: There was some w/a described somewhere suggesting loading > * contexts before PPGTT. > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index 3a8bf0c..b07bdfb 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -204,7 +204,8 @@ i915_l3_write(struct file *filp, struct kobject *kobj, > > memcpy(dev_priv->l3_parity.remap_info[slice] + (offset/4), buf, count); > > - i915_gem_l3_remap(drm_dev, slice); > + if (i915_gem_l3_remap(&dev_priv->ring[RCS], slice)) > + count = 0; > > mutex_unlock(&drm_dev->struct_mutex); > > -- > 1.8.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch