Re: A17 tiling series V2
On Wed, 2009-04-08 at 19:41 +0100, Chris Wilson wrote:
> On Wed, 2009-04-08 at 11:05 -0700, Eric Anholt wrote:
> > Here's the pair of patches I've got to make A17-afflicted machines do
> > tiling.
> > I've created regression tests for swapping tiled objects and for pread on
> > tiled objects, and I'm at 1/10 failures in intel-gpu-tools, or 0/10 if I
> > disable BO reuse (appears to be a bug in fencing on objects getting reused
> > as tiled after being linear). 3D performance is back to normal on my
> > affected 945GM, and no_rast=true glxgears looks good.
>
> Sounds like this patch should do the trick...
I don't think that's what's going on with the issue I was seeing, since
strides are all the same, and it's an untiled -> tiled transition
failing (I think). But I can't say for sure.
On first glance, this patch tricked me because I was assuming that
i915_gem_tiling_ok was "did the tiling mode we choose end up being OK
for this bo", not "is the current GTT location OK for the new tiling
mode that we know for sure is OK?". I'm tempted to rename it.
Given that the current driver stack doesn't do any tiled -> tiled
transitions, I'm thinking of holding off on this one until the next
merge window. Any strong objections?
> >From 1c5d1156f0f467568b1eab18a33dd9edc3579ce5 Mon Sep 17 00:00:00 2001
> From: Chris Wilson
> Date: Wed, 11 Feb 2009 10:45:16 +
> Subject: [PATCH] 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
> ---
> 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 3784cfc..833ad26 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -628,6 +628,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 5848c1d..43c71de 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2019,7 +2019,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);
> -
> }
>
> /**
> @@ -2211,6 +2210,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;
> +}
> +
> +/**
> * 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 6be3f92..373e457 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -255,6 +255,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_TILIN
Re: A17 tiling series V2
On Wed, 2009-04-08 at 11:05 -0700, Eric Anholt wrote:
> Here's the pair of patches I've got to make A17-afflicted machines do tiling.
> I've created regression tests for swapping tiled objects and for pread on
> tiled objects, and I'm at 1/10 failures in intel-gpu-tools, or 0/10 if I
> disable BO reuse (appears to be a bug in fencing on objects getting reused
> as tiled after being linear). 3D performance is back to normal on my
> affected 945GM, and no_rast=true glxgears looks good.
Sounds like this patch should do the trick...
>From 1c5d1156f0f467568b1eab18a33dd9edc3579ce5 Mon Sep 17 00:00:00 2001
From: Chris Wilson
Date: Wed, 11 Feb 2009 10:45:16 +
Subject: [PATCH] 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
---
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 3784cfc..833ad26 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -628,6 +628,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 5848c1d..43c71de 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2019,7 +2019,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);
-
}
/**
@@ -2211,6 +2210,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;
+}
+
+/**
* 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 6be3f92..373e457 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -255,6 +255,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.
@@ -267,6 +282,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)
@@ -274,15 +290,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)
A17 tiling series V2
Here's the pair of patches I've got to make A17-afflicted machines do tiling. I've created regression tests for swapping tiled objects and for pread on tiled objects, and I'm at 1/10 failures in intel-gpu-tools, or 0/10 if I disable BO reuse (appears to be a bug in fencing on objects getting reused as tiled after being linear). 3D performance is back to normal on my affected 945GM, and no_rast=true glxgears looks good. I haven't written a regression test for tiled pwrite yet, is my only concern. -- This SF.net email is sponsored by: High Quality Requirements in a Collaborative Environment. Download a free trial of Rational Requirements Composer Now! http://p.sf.net/sfu/www-ibm-com -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
