From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 8/8] drm/i915: Do remaps for all contexts Date: Fri, 13 Sep 2013 12:17:17 +0300 Message-ID: <20130913091717.GL20128@intel.com> References: <1379050122-12774-1-git-send-email-benjamin.widawsky@intel.com> <1379050122-12774-9-git-send-email-benjamin.widawsky@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 56752E5CD6 for ; Fri, 13 Sep 2013 02:17:21 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1379050122-12774-9-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@lists.freedesktop.org, vishnu.venkatesh@intel.com, bryan.j.bell@intel.com, Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Thu, Sep 12, 2013 at 10:28:34PM -0700, Ben Widawsky wrote: > On both Ivybridge and Haswell, row remapping information is saved and > restored with context. This means, we never actually properly supported > the l3 remapping because our sysfs interface is asynchronous (and not > tied to any context), and the known faulty HW would be reused by the > next context to run. > = > Not that due to the asynchronous nature of the sysfs entry, there is no > point modifying the registers for the existing context. Instead we set a > flag for all contexts to load the correct remapping information on the > next run. Interested clients can use debugfs to determine whether or not > the row has been remapped. > = > One could propose at this point that we just do the remapping in the > kernel. I guess since we have to maintain the sysfs interface anyway, > I'm not sure how useful it is, and I do like keeping the policy in > userspace; (it wasn't my original decision to make the > interface the way it is, so I'm not attached). > = > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_debugfs.c | 8 +++++++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_context.c | 17 +++++++++++++-- > drivers/gpu/drm/i915/i915_sysfs.c | 38 +++++++++++----------------= ------ > 4 files changed, 36 insertions(+), 28 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i= 915_debugfs.c > index ada0950..80bed69 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -145,6 +145,13 @@ describe_obj(struct seq_file *m, struct drm_i915_gem= _object *obj) > seq_printf(m, " (%s)", obj->ring->name); > } > = > +static void describe_ctx(struct seq_file *m, struct i915_hw_context *ctx) > +{ > + seq_putc(m, ctx->is_initialized ? 'I' : 'i'); > + seq_putc(m, ctx->remap_slice ? 'R' : 'r'); > + seq_putc(m, ' '); > +} > + > static int i915_gem_object_list_info(struct seq_file *m, void *data) > { > struct drm_info_node *node =3D (struct drm_info_node *) m->private; > @@ -1463,6 +1470,7 @@ static int i915_context_status(struct seq_file *m, = void *unused) > = > list_for_each_entry(ctx, &dev_priv->context_list, link) { > seq_puts(m, "HW context "); > + describe_ctx(m, ctx); > for_each_ring(ring, dev_priv, i) > if (ring->default_context =3D=3D ctx) > seq_printf(m, "(default context %s) ", ring->name); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index 68f992b..6ba78cd 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -602,6 +602,7 @@ struct i915_hw_context { > struct kref ref; > int id; > bool is_initialized; > + uint8_t remap_slice; > struct drm_i915_file_private *file_priv; > struct intel_ring_buffer *ring; > struct drm_i915_gem_object *obj; > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i9= 15/i915_gem_context.c > index 2bbdce8..72b7202 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -140,7 +140,7 @@ create_hw_context(struct drm_device *dev, > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > struct i915_hw_context *ctx; > - int ret; > + int ret, i; > = > ctx =3D kzalloc(sizeof(*ctx), GFP_KERNEL); > if (ctx =3D=3D NULL) > @@ -181,6 +181,8 @@ create_hw_context(struct drm_device *dev, > = > ctx->file_priv =3D file_priv; > ctx->id =3D ret; > + for (i =3D 0; i < NUM_L3_SLICES(dev); i++) > + ctx->remap_slice |=3D 1 << 1; ^ i > = > return ctx; > = > @@ -396,7 +398,7 @@ static int do_switch(struct i915_hw_context *to) > struct intel_ring_buffer *ring =3D to->ring; > struct i915_hw_context *from =3D ring->last_context; > u32 hw_flags =3D 0; > - int ret; > + int ret, i; > = > BUG_ON(from !=3D NULL && from->obj !=3D NULL && from->obj->pin_count = =3D=3D 0); > = > @@ -432,6 +434,17 @@ static int do_switch(struct i915_hw_context *to) > return ret; > } > = > + for (i =3D 0; i < MAX_L3_SLICES; i++) { > + if (!(to->remap_slice & (1< + continue; > + > + ret =3D i915_gem_l3_remap(ring, i); > + if (!ret) { > + to->remap_slice &=3D ~(1< + } > + /* If it failed, try again next round */ > + } > + > /* The backing object for the context is done after switching to the > * *next* context. Therefore we cannot retire the previous context until > * the next context has already started running. In fact, the below code > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i91= 5_sysfs.c > index 65a7274..f0d7e1c 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -118,9 +118,8 @@ i915_l3_read(struct file *filp, struct kobject *kobj, > struct drm_minor *dminor =3D container_of(dev, struct drm_minor, kdev); > struct drm_device *drm_dev =3D dminor->dev; > struct drm_i915_private *dev_priv =3D drm_dev->dev_private; > - uint32_t misccpctl; > int slice =3D (int)(uintptr_t)attr->private; > - int i, ret; > + int ret; > = > count =3D round_down(count, 4); > = > @@ -134,31 +133,16 @@ i915_l3_read(struct file *filp, struct kobject *kob= j, > if (ret) > return ret; > = > - if (IS_HASWELL(drm_dev)) { > - int last =3D min_t(int, GEN7_L3LOG_SIZE, count + offset); > - if ((!dev_priv->l3_parity.remap_info[slice])) > - memset(buf + offset, 0, last - offset); > - else > - memcpy(buf + offset, > - dev_priv->l3_parity.remap_info[slice] + (offset/4), > - last - offset); > - > - i =3D last; > - goto out; > - } > - > - misccpctl =3D I915_READ(GEN7_MISCCPCTL); > - I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE); > - > - for (i =3D 0; i < count; i +=3D 4) > - *((uint32_t *)(&buf[i])) =3D I915_READ(GEN7_L3LOG_BASE + offset + i); > - > - I915_WRITE(GEN7_MISCCPCTL, misccpctl); > + if ((!dev_priv->l3_parity.remap_info[slice])) > + memset(buf + offset, 0, count); > + else > + memcpy(buf + offset, > + dev_priv->l3_parity.remap_info[slice] + (offset/4), > + count); Needs fixing after patch 4 is fixed. > = > -out: > mutex_unlock(&drm_dev->struct_mutex); > = > - return i; > + return count; > } > = > static ssize_t > @@ -170,6 +154,7 @@ i915_l3_write(struct file *filp, struct kobject *kobj, > struct drm_minor *dminor =3D container_of(dev, struct drm_minor, kdev); > struct drm_device *drm_dev =3D dminor->dev; > struct drm_i915_private *dev_priv =3D drm_dev->dev_private; > + struct i915_hw_context *ctx; > u32 *temp =3D NULL; /* Just here to make handling failures easy */ > int slice =3D (int)(uintptr_t)attr->private; > int ret; > @@ -206,8 +191,9 @@ i915_l3_write(struct file *filp, struct kobject *kobj, > = > memcpy(dev_priv->l3_parity.remap_info[slice] + (offset/4), buf, count); > = > - if (i915_gem_l3_remap(&dev_priv->ring[RCS], slice)) > - count =3D 0; > + /* NB: We defer the remapping until we switch to the context */ > + list_for_each_entry(ctx, &dev_priv->context_list, link) > + ctx->remap_slice |=3D (1< = > 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 -- = Ville Syrj=E4l=E4 Intel OTC