Re: [Intel-gfx] [CI 4/6] drm/i915: Always use the GTT for error capture
On Thu, Oct 13, 2016 at 05:02:50PM +0200, Daniel Vetter wrote: > On Wed, Oct 12, 2016 at 10:05:20AM +0100, Chris Wilson wrote: > > Since the GTT provides universal access to any GPU page, we can use it > > to reduce our plethora of read methods to just one. It also has the > > important characteristic of being exactly what the GPU sees - if there > > are incoherency problems, seeing the batch as executed (rather than as > > trapped inside the cpu cache) is important. > > > > Signed-off-by: Chris Wilson > > Reviewed-by: Joonas Lahtinen > > Meh, so much for me reading patches in order ;-) > > Please reorder this one before the previous one and I'll be happy too. Hmm, I actually thought this was. Shows what I remember about my series. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 4/6] drm/i915: Always use the GTT for error capture
On Wed, Oct 12, 2016 at 10:05:20AM +0100, Chris Wilson wrote:
> Since the GTT provides universal access to any GPU page, we can use it
> to reduce our plethora of read methods to just one. It also has the
> important characteristic of being exactly what the GPU sees - if there
> are incoherency problems, seeing the batch as executed (rather than as
> trapped inside the cpu cache) is important.
>
> Signed-off-by: Chris Wilson
> Reviewed-by: Joonas Lahtinen
Meh, so much for me reading patches in order ;-)
Please reorder this one before the previous one and I'll be happy too.
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 43
> drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +
> drivers/gpu/drm/i915/i915_gpu_error.c | 120
> --
> 3 files changed, 74 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 0bb4232f66bc..2d846aa39ca5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2717,6 +2717,7 @@ int i915_gem_init_ggtt(struct drm_i915_private
> *dev_priv)
>*/
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> unsigned long hole_start, hole_end;
> + struct i915_hw_ppgtt *ppgtt;
> struct drm_mm_node *entry;
> int ret;
>
> @@ -2724,6 +2725,15 @@ int i915_gem_init_ggtt(struct drm_i915_private
> *dev_priv)
> if (ret)
> return ret;
>
> + /* Reserve a mappable slot for our lockless error capture */
> + ret = drm_mm_insert_node_in_range_generic(&ggtt->base.mm,
> + &ggtt->error_capture,
> + 4096, 0, -1,
> + 0, ggtt->mappable_end,
> + 0, 0);
> + if (ret)
> + return ret;
> +
> /* Clear any non-preallocated blocks */
> drm_mm_for_each_hole(entry, &ggtt->base.mm, hole_start, hole_end) {
> DRM_DEBUG_KMS("clearing unused GTT space: [%lx, %lx]\n",
> @@ -2738,25 +2748,21 @@ int i915_gem_init_ggtt(struct drm_i915_private
> *dev_priv)
> true);
>
> if (USES_PPGTT(dev_priv) && !USES_FULL_PPGTT(dev_priv)) {
> - struct i915_hw_ppgtt *ppgtt;
> -
> ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
> - if (!ppgtt)
> - return -ENOMEM;
> + if (!ppgtt) {
> + ret = -ENOMEM;
> + goto err;
> + }
>
> ret = __hw_ppgtt_init(ppgtt, dev_priv);
> - if (ret) {
> - kfree(ppgtt);
> - return ret;
> - }
> + if (ret)
> + goto err_ppgtt;
>
> - if (ppgtt->base.allocate_va_range)
> + if (ppgtt->base.allocate_va_range) {
> ret = ppgtt->base.allocate_va_range(&ppgtt->base, 0,
> ppgtt->base.total);
> - if (ret) {
> - ppgtt->base.cleanup(&ppgtt->base);
> - kfree(ppgtt);
> - return ret;
> + if (ret)
> + goto err_ppgtt_cleanup;
> }
>
> ppgtt->base.clear_range(&ppgtt->base,
> @@ -2770,6 +2776,14 @@ int i915_gem_init_ggtt(struct drm_i915_private
> *dev_priv)
> }
>
> return 0;
> +
> +err_ppgtt_cleanup:
> + ppgtt->base.cleanup(&ppgtt->base);
> +err_ppgtt:
> + kfree(ppgtt);
> +err:
> + drm_mm_remove_node(&ggtt->error_capture);
> + return ret;
> }
>
> /**
> @@ -2788,6 +2802,9 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private
> *dev_priv)
>
> i915_gem_cleanup_stolen(&dev_priv->drm);
>
> + if (drm_mm_node_allocated(&ggtt->error_capture))
> + drm_mm_remove_node(&ggtt->error_capture);
> +
> if (drm_mm_initialized(&ggtt->base.mm)) {
> intel_vgt_deballoon(dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
> b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index ec78be2f8c77..bd93fb8f99d2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -450,6 +450,8 @@ struct i915_ggtt {
> bool do_idle_maps;
>
> int mtrr;
> +
> + struct drm_mm_node error_capture;
> };
>
> struct i915_hw_ppgtt {
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 159d6d7e0cee..b3b2e6c1c6c6 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -656,7 +656,7 @@ static void i915_error_object_free(struct
> drm_i915_error_object *obj)
> return;
>
> for (page = 0; page < obj->page_count; page++)
> - kfree(obj->pages[page]);
> +
[Intel-gfx] [CI 4/6] drm/i915: Always use the GTT for error capture
Since the GTT provides universal access to any GPU page, we can use it
to reduce our plethora of read methods to just one. It also has the
important characteristic of being exactly what the GPU sees - if there
are incoherency problems, seeing the batch as executed (rather than as
trapped inside the cpu cache) is important.
Signed-off-by: Chris Wilson
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 43
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +
drivers/gpu/drm/i915/i915_gpu_error.c | 120 --
3 files changed, 74 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0bb4232f66bc..2d846aa39ca5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2717,6 +2717,7 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
*/
struct i915_ggtt *ggtt = &dev_priv->ggtt;
unsigned long hole_start, hole_end;
+ struct i915_hw_ppgtt *ppgtt;
struct drm_mm_node *entry;
int ret;
@@ -2724,6 +2725,15 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
if (ret)
return ret;
+ /* Reserve a mappable slot for our lockless error capture */
+ ret = drm_mm_insert_node_in_range_generic(&ggtt->base.mm,
+ &ggtt->error_capture,
+ 4096, 0, -1,
+ 0, ggtt->mappable_end,
+ 0, 0);
+ if (ret)
+ return ret;
+
/* Clear any non-preallocated blocks */
drm_mm_for_each_hole(entry, &ggtt->base.mm, hole_start, hole_end) {
DRM_DEBUG_KMS("clearing unused GTT space: [%lx, %lx]\n",
@@ -2738,25 +2748,21 @@ int i915_gem_init_ggtt(struct drm_i915_private
*dev_priv)
true);
if (USES_PPGTT(dev_priv) && !USES_FULL_PPGTT(dev_priv)) {
- struct i915_hw_ppgtt *ppgtt;
-
ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
- if (!ppgtt)
- return -ENOMEM;
+ if (!ppgtt) {
+ ret = -ENOMEM;
+ goto err;
+ }
ret = __hw_ppgtt_init(ppgtt, dev_priv);
- if (ret) {
- kfree(ppgtt);
- return ret;
- }
+ if (ret)
+ goto err_ppgtt;
- if (ppgtt->base.allocate_va_range)
+ if (ppgtt->base.allocate_va_range) {
ret = ppgtt->base.allocate_va_range(&ppgtt->base, 0,
ppgtt->base.total);
- if (ret) {
- ppgtt->base.cleanup(&ppgtt->base);
- kfree(ppgtt);
- return ret;
+ if (ret)
+ goto err_ppgtt_cleanup;
}
ppgtt->base.clear_range(&ppgtt->base,
@@ -2770,6 +2776,14 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
}
return 0;
+
+err_ppgtt_cleanup:
+ ppgtt->base.cleanup(&ppgtt->base);
+err_ppgtt:
+ kfree(ppgtt);
+err:
+ drm_mm_remove_node(&ggtt->error_capture);
+ return ret;
}
/**
@@ -2788,6 +2802,9 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private
*dev_priv)
i915_gem_cleanup_stolen(&dev_priv->drm);
+ if (drm_mm_node_allocated(&ggtt->error_capture))
+ drm_mm_remove_node(&ggtt->error_capture);
+
if (drm_mm_initialized(&ggtt->base.mm)) {
intel_vgt_deballoon(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index ec78be2f8c77..bd93fb8f99d2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -450,6 +450,8 @@ struct i915_ggtt {
bool do_idle_maps;
int mtrr;
+
+ struct drm_mm_node error_capture;
};
struct i915_hw_ppgtt {
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 159d6d7e0cee..b3b2e6c1c6c6 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -656,7 +656,7 @@ static void i915_error_object_free(struct
drm_i915_error_object *obj)
return;
for (page = 0; page < obj->page_count; page++)
- kfree(obj->pages[page]);
+ free_page((unsigned long)obj->pages[page]);
kfree(obj);
}
@@ -693,98 +693,69 @@ static void i915_error_state_free(struct kref *error_ref)
kfree(error);
}
+static int compress_page(void *src, struct drm_i915_error_object *dst)
+{
+ unsigned long page;
+
+ page = __get_free_page(GFP_ATOMIC | __
