[Intel-gfx] [PATCH] drm/i915: Clear fence register on tiling stride change.
Eric Anholt
eric at anholt.net
Fri Feb 13 20:55:01 CET 2009
On Wed, 2009-02-11 at 22:12 +0000, Chris Wilson wrote:
> On Wed, 2009-02-11 at 14:26 +0000, Chris Wilson wrote:
> > The fence register value also depends upon the stride of the object, so we
> > need to clear the fence if that is changed as well.
>
> The deliberate mistake I made here was to forget about the tiling
> alignment restrictions when changing tiling_mode. Updated patch follows.
>
> >From 2d1440d8abf5f0addd1f3d33aa57b4b8f4b70144 Mon Sep 17 00:00:00 2001
> From: Chris Wilson <chris at chris-wilson.co.uk>
> Date: Wed, 11 Feb 2009 10:45:16 +0000
> Subject: [PATCH 22/22] drm/i915: Clear fence register on tiling stride change.
>
> The fence register value also depends upon the stride of the object, so we
> need to clear the fence if that is changed as well.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_gem.c | 37 +++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_gem_tiling.c | 54 ++++++++++++++++++++++---------
> 3 files changed, 75 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3151da4..1084b03 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -618,6 +618,7 @@ void i915_gem_object_unpin(struct drm_gem_object *obj);
> int i915_gem_object_unbind(struct drm_gem_object *obj);
> void i915_gem_lastclose(struct drm_device *dev);
> uint32_t i915_get_gem_seqno(struct drm_device *dev);
> +int i915_gem_object_put_fence_reg(struct drm_gem_object *obj);
> void i915_gem_retire_requests(struct drm_device *dev);
> void i915_gem_retire_work_handler(struct work_struct *work);
> void i915_gem_clflush_object(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6da3a6c..e30ee59 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1520,7 +1520,6 @@ static void i830_write_fence_reg(struct drm_i915_fence_reg *reg)
> val |= I830_FENCE_REG_VALID;
>
> I915_WRITE(FENCE_REG_830_0 + (regnum * 4), val);
> -
> }
>
> /**
> @@ -1704,6 +1703,42 @@ i915_gem_clear_fence_reg(struct drm_gem_object *obj)
> }
>
> /**
> + * i915_gem_object_put_fence_reg - waits on outstanding fenced access
> + * to the buffer to finish, and then resets the fence register.
> + * @obj: tiled object holding a fence register.
> + *
> + * Zeroes out the fence register itself and clears out the associated
> + * data structures in dev_priv and obj_priv.
> + */
> +int
> +i915_gem_object_put_fence_reg(struct drm_gem_object *obj)
> +{
> + struct drm_device *dev = obj->dev;
> + struct drm_i915_gem_object *obj_priv = obj->driver_private;
> +
> + if (obj_priv->fence_reg == I915_FENCE_REG_NONE)
> + return 0;
> +
> + /* On the i915, GPU access to tiled buffers is via a fence,
> + * therefore we must wait for any outstanding access to complete
> + * before clearing the fence.
> + */
> + if (!IS_I965G(dev)) {
> + int ret;
> +
> + i915_gem_object_flush_gpu_write_domain(obj);
> + i915_gem_object_flush_gtt_write_domain(obj);
> + ret = i915_gem_object_wait_rendering(obj);
> + if (ret != 0)
> + return ret;
> + }
> +
> + i915_gem_clear_fence_reg (obj);
> +
> + return 0;
> +}
> +
Thanks for bringing this into a separate function -- it's needed for the
fence stealing that's broken (as far as I can tell) in get_fence_reg as
well. I think you'll also need clearing of the read domain flags here,
though, as you don't want read caches containing the old-strided data.
(I'm assuming here that fence address translation applies between the
GPU caches and the aperture, not between the GPU and the GPU caches. I
don't think we have any concrete information confirming that, though.)
> +/**
> * Finds free space in the GTT aperture and binds the object there.
> */
> static int
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index 7fb4191..908e99e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -240,6 +240,21 @@ i915_tiling_ok(struct drm_device *dev, int stride, int size, int tiling_mode)
> return true;
> }
>
> +static bool
> +i915_gem_object_tiling_ok (struct drm_gem_object *obj, int tiling_mode)
> +{
> + struct drm_i915_gem_object *obj_priv = obj->driver_private;
> +
> + if (obj_priv->gtt_space == NULL)
> + return true;
> +
> + if (tiling_mode == I915_TILING_NONE)
> + return true;
> +
> + return (obj_priv->gtt_offset & ~I915_FENCE_START_MASK) == 0 &&
> + (obj_priv->gtt_offset & (obj->size - 1)) == 0;
> +}
> +
> /**
> * Sets the tiling mode of an object, returning the required swizzling of
> * bit 6 of addresses in the object.
> @@ -252,6 +267,7 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
> drm_i915_private_t *dev_priv = dev->dev_private;
> struct drm_gem_object *obj;
> struct drm_i915_gem_object *obj_priv;
> + int ret = 0;
>
> obj = drm_gem_object_lookup(dev, file_priv, args->handle);
> if (obj == NULL)
> @@ -259,15 +275,15 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
> obj_priv = obj->driver_private;
>
> if (!i915_tiling_ok(dev, args->stride, obj->size, args->tiling_mode)) {
> + mutex_lock(&dev->struct_mutex);
> drm_gem_object_unreference(obj);
> + mutex_unlock(&dev->struct_mutex);
> return -EINVAL;
> }
>
> - mutex_lock(&dev->struct_mutex);
> -
> if (args->tiling_mode == I915_TILING_NONE) {
> - obj_priv->tiling_mode = I915_TILING_NONE;
> args->swizzle_mode = I915_BIT_6_SWIZZLE_NONE;
> + args->stride = 0;
> } else {
> if (args->tiling_mode == I915_TILING_X)
> args->swizzle_mode = dev_priv->mm.bit_6_swizzle_x;
> @@ -277,32 +293,38 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
> if (args->swizzle_mode == I915_BIT_6_SWIZZLE_UNKNOWN) {
> args->tiling_mode = I915_TILING_NONE;
> args->swizzle_mode = I915_BIT_6_SWIZZLE_NONE;
> + args->stride = 0;
> }
> }
> - if (args->tiling_mode != obj_priv->tiling_mode) {
> - int ret;
>
> - /* Unbind the object, as switching tiling means we're
> - * switching the cache organization due to fencing, probably.
> + mutex_lock(&dev->struct_mutex);
> + if (args->tiling_mode != obj_priv->tiling_mode ||
> + args->stride != obj_priv->stride) {
> + /* We need to rebind the object if its current allocation
> + * no longer meets the alignment restrictions for its new
> + * tiling mode. Otherwise we can just leave it alone, but
> + * need to ensure that any fence register is cleared.
> */
> - ret = i915_gem_object_unbind(obj);
> + if (!i915_gem_object_tiling_ok(obj, args->tiling_mode))
> + ret = i915_gem_object_unbind(obj);
> + else
> + ret = i915_gem_object_put_fence_reg(obj);
> if (ret != 0) {
> WARN(ret != -ERESTARTSYS,
> - "failed to unbind object for tiling switch");
> + "failed to reset object for tiling switch");
> args->tiling_mode = obj_priv->tiling_mode;
> - mutex_unlock(&dev->struct_mutex);
> - drm_gem_object_unreference(obj);
> -
> - return ret;
> + args->stride = obj_priv->stride;
> + goto err;
> }
> +
> obj_priv->tiling_mode = args->tiling_mode;
> + obj_priv->stride = args->stride;
> }
> - obj_priv->stride = args->stride;
> -
> +err:
> drm_gem_object_unreference(obj);
> mutex_unlock(&dev->struct_mutex);
>
> - return 0;
> + return ret;
> }
>
> /**
--
Eric Anholt
eric at anholt.net eric.anholt at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090213/8d445f15/attachment.sig>
More information about the Intel-gfx
mailing list