Quoting Matthew Auld (2019-08-09 23:26:32)
> From: Daniele Ceraolo Spurio <daniele.ceraolospu...@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.ceraolospu...@intel.com>
> Signed-off-by: Abdiel Janulgue <abdiel.janul...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
