[PATCH v3 26/37] drm/i915: error capture with no ggtt slot

Chris Wilson chris at chris-wilson.co.uk
Sat Aug 10 11:11:46 UTC 2019


Quoting Matthew Auld (2019-08-09 23:26:32)
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> 
> If the aperture is not available in HW we can't use a ggtt slot and wc
> copy, so fall back to regular kmap.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c   | 19 ++++----
>  drivers/gpu/drm/i915/i915_gpu_error.c | 64 ++++++++++++++++++++++-----
>  2 files changed, 63 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index dd28c54527e3..0819ac9837dc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2630,7 +2630,8 @@ static void ggtt_release_guc_top(struct i915_ggtt *ggtt)
>  static void cleanup_init_ggtt(struct i915_ggtt *ggtt)
>  {
>         ggtt_release_guc_top(ggtt);
> -       drm_mm_remove_node(&ggtt->error_capture);
> +       if (drm_mm_node_allocated(&ggtt->error_capture))
> +               drm_mm_remove_node(&ggtt->error_capture);
>  }
>  
>  static int init_ggtt(struct i915_ggtt *ggtt)
> @@ -2661,13 +2662,15 @@ static int init_ggtt(struct i915_ggtt *ggtt)
>         if (ret)
>                 return ret;
>  
> -       /* Reserve a mappable slot for our lockless error capture */
> -       ret = drm_mm_insert_node_in_range(&ggtt->vm.mm, &ggtt->error_capture,
> -                                         PAGE_SIZE, 0, I915_COLOR_UNEVICTABLE,
> -                                         0, ggtt->mappable_end,
> -                                         DRM_MM_INSERT_LOW);
> -       if (ret)
> -               return ret;
> +       if (HAS_MAPPABLE_APERTURE(ggtt->vm.i915)) {
> +               /* Reserve a mappable slot for our lockless error capture */
> +               ret = drm_mm_insert_node_in_range(&ggtt->vm.mm, &ggtt->error_capture,
> +                                                 PAGE_SIZE, 0, I915_COLOR_UNEVICTABLE,
> +                                                 0, ggtt->mappable_end,
> +                                                 DRM_MM_INSERT_LOW);
> +               if (ret)
> +                       return ret;
> +       }
>  
>         /*
>          * The upper portion of the GuC address space has a sizeable hole
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 92986d3f6995..19eb5ccba387 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -40,6 +40,7 @@
>  #include "display/intel_overlay.h"
>  
>  #include "gem/i915_gem_context.h"
> +#include "gem/i915_gem_lmem.h"
>  
>  #include "i915_drv.h"
>  #include "i915_gpu_error.h"
> @@ -235,6 +236,7 @@ struct compress {
>         struct pagevec pool;
>         struct z_stream_s zstream;
>         void *tmp;
> +       bool wc;
>  };
>  
>  static bool compress_init(struct compress *c)
> @@ -292,7 +294,7 @@ static int compress_page(struct compress *c,
>         struct z_stream_s *zstream = &c->zstream;
>  
>         zstream->next_in = src;
> -       if (c->tmp && i915_memcpy_from_wc(c->tmp, src, PAGE_SIZE))
> +       if (c->wc && c->tmp && i915_memcpy_from_wc(c->tmp, src, PAGE_SIZE))
>                 zstream->next_in = c->tmp;
>         zstream->avail_in = PAGE_SIZE;
>  
> @@ -367,6 +369,7 @@ static void err_compression_marker(struct drm_i915_error_state_buf *m)
>  
>  struct compress {
>         struct pagevec pool;
> +       bool wc;
>  };
>  
>  static bool compress_init(struct compress *c)
> @@ -389,7 +392,7 @@ static int compress_page(struct compress *c,
>         if (!ptr)
>                 return -ENOMEM;
>  
> -       if (!i915_memcpy_from_wc(ptr, src, PAGE_SIZE))
> +       if (!(c->wc && i915_memcpy_from_wc(ptr, src, PAGE_SIZE)))
>                 memcpy(ptr, src, PAGE_SIZE);
>         dst->pages[dst->page_count++] = ptr;
>  
> @@ -963,7 +966,6 @@ i915_error_object_create(struct drm_i915_private *i915,
>         struct drm_i915_error_object *dst;
>         unsigned long num_pages;
>         struct sgt_iter iter;
> -       dma_addr_t dma;
>         int ret;
>  
>         might_sleep();
> @@ -988,17 +990,53 @@ i915_error_object_create(struct drm_i915_private *i915,
>         dst->page_count = 0;
>         dst->unused = 0;
>  
> +       compress->wc = i915_gem_object_is_lmem(vma->obj) ||

We need to talk about this fixation you appear to have on vma->obj!

> +                      drm_mm_node_allocated(&ggtt->error_capture);
> +
>         ret = -EINVAL;
> -       for_each_sgt_dma(dma, iter, vma->pages) {
> +       if (drm_mm_node_allocated(&ggtt->error_capture)) {
>                 void __iomem *s;
> +               dma_addr_t dma;
>  
> -               ggtt->vm.insert_page(&ggtt->vm, dma, slot, I915_CACHE_NONE, 0);
> +               for_each_sgt_dma(dma, iter, vma->pages) {
> +                       ggtt->vm.insert_page(&ggtt->vm, dma, slot,
> +                                            I915_CACHE_NONE, 0);
>  
> -               s = io_mapping_map_wc(&ggtt->iomap, slot, PAGE_SIZE);
> -               ret = compress_page(compress, (void  __force *)s, dst);
> -               io_mapping_unmap(s);
> -               if (ret)
> -                       break;
> +                       s = io_mapping_map_wc(&ggtt->iomap, slot, PAGE_SIZE);
> +                       ret = compress_page(compress, (void  __force *)s, dst);
> +                       io_mapping_unmap(s);
> +                       if (ret)
> +                               break;
> +               }
> +       } else if (i915_gem_object_is_lmem(vma->obj)) {
> +               void *s;
> +               dma_addr_t dma;
> +               struct intel_memory_region *mem = vma->obj->mm.region;

You did that on purpose!

> +
> +               for_each_sgt_dma(dma, iter, vma->pages) {
> +                       s = io_mapping_map_atomic_wc(&mem->iomap, dma);
> +                       ret = compress_page(compress, s, dst);
> +                       io_mapping_unmap_atomic(s);
> +
> +                       if (ret)
> +                               break;
> +               }
> +       } else {
> +               void *s;
> +               struct page *page;
> +
> +               for_each_sgt_page(page, iter, vma->pages) {
void *s;

Then I wouldn't even complain about the ordering.

> +                       drm_clflush_pages(&page, 1);
> +
> +                       s = kmap_atomic(page);
> +                       ret = compress_page(compress, s, dst);
> +                       kunmap_atomic(s);
> +
> +                       if (ret)
> +                               break;
> +
> +                       drm_clflush_pages(&page, 1);

clflush before the ret

I have to say the circle is complete.
-Chris


More information about the dri-devel mailing list