Re: [Intel-gfx] [PATCH 1/2] drm/i915: drop lrc header page

2019-10-31 Thread Matthew Brost

On Wed, Oct 30, 2019 at 06:30:39PM -0700, Daniele Ceraolo Spurio wrote:

Recent GuC binaries (including all the ones we're currently using)
don't require this shared area anymore, having moved the relevant
entries into the stage pool instead. i915 itself doesn't write
anything into it either, so we can safely drop it.

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Chris Wilson 
Cc: Michal Wajdeczko 
Cc: John Harrison 
Cc: Matthew Brost 
---
drivers/gpu/drm/i915/gt/intel_lrc.c| 22 +++--
drivers/gpu/drm/i915/gt/intel_lrc.h| 23 ++
drivers/gpu/drm/i915/gt/selftest_context.c |  3 ---
drivers/gpu/drm/i915/gvt/scheduler.c   |  4 ++--
4 files changed, 7 insertions(+), 45 deletions(-)



This patch works with the GuC redesign work being done.

Reviewed-by: Matthew Brost 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: drop lrc header page

2019-10-31 Thread Chris Wilson
Quoting Daniele Ceraolo Spurio (2019-10-31 01:30:39)
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h 
> b/drivers/gpu/drm/i915/gt/intel_lrc.h
> index faa2d56c279b..04511d8ebdc1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.h
> @@ -86,31 +86,12 @@ int intel_execlists_submission_setup(struct 
> intel_engine_cs *engine);
>  int intel_execlists_submission_init(struct intel_engine_cs *engine);
>  
>  /* Logical Ring Contexts */
> -
> -/*
> - * We allocate a header at the start of the context image for our own
> - * use, therefore the actual location of the logical state is offset
> - * from the start of the VMA. The layout is
> - *
> - * | [guc]  | [hwsp] [logical state] |
> - * |<- our header ->|<- context image  ->|
> - *
> - */
> -/* The first page is used for sharing data with the GuC */
> -#define LRC_GUCSHR_PN  (0)
> -#define LRC_GUCSHR_SZ  (1)
>  /* At the start of the context image is its per-process HWS page */
> -#define LRC_PPHWSP_PN  (LRC_GUCSHR_PN + LRC_GUCSHR_SZ)
> +#define LRC_PPHWSP_PN  (0)
>  #define LRC_PPHWSP_SZ  (1)
> -/* Finally we have the logical state for the context */
> +/* After the PPHWSP we have the logical state for the context */
>  #define LRC_STATE_PN   (LRC_PPHWSP_PN + LRC_PPHWSP_SZ)
>  
> -/*
> - * Currently we include the PPHWSP in __intel_engine_context_size() so
> - * the size of the header is synonymous with the start of the PPHWSP.
> - */
> -#define LRC_HEADER_PAGES LRC_PPHWSP_PN

Acked-by: Chris Wilson 
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/2] drm/i915: drop lrc header page

2019-10-30 Thread Daniele Ceraolo Spurio
Recent GuC binaries (including all the ones we're currently using)
don't require this shared area anymore, having moved the relevant
entries into the stage pool instead. i915 itself doesn't write
anything into it either, so we can safely drop it.

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Chris Wilson 
Cc: Michal Wajdeczko 
Cc: John Harrison 
Cc: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/intel_lrc.c| 22 +++--
 drivers/gpu/drm/i915/gt/intel_lrc.h| 23 ++
 drivers/gpu/drm/i915/gt/selftest_context.c |  3 ---
 drivers/gpu/drm/i915/gvt/scheduler.c   |  4 ++--
 4 files changed, 7 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 6fb3def5ba16..51aef2a233cb 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -471,8 +471,7 @@ lrc_descriptor(struct intel_context *ce, struct 
intel_engine_cs *engine)
if (IS_GEN(engine->i915, 8))
desc |= GEN8_CTX_L3LLC_COHERENT;
 
-   desc |= i915_ggtt_offset(ce->state) + LRC_HEADER_PAGES * PAGE_SIZE;
-   /* bits 12-31 */
+   desc |= i915_ggtt_offset(ce->state); /* bits 12-31 */
/*
 * The following 32bits are copied into the OA reports (dword 2).
 * Consider updating oa_get_render_ctx_id in i915_perf.c when changing
@@ -2316,7 +2315,6 @@ set_redzone(void *vaddr, const struct intel_engine_cs 
*engine)
if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
return;
 
-   vaddr += LRC_HEADER_PAGES * PAGE_SIZE;
vaddr += engine->context_size;
 
memset(vaddr, POISON_INUSE, I915_GTT_PAGE_SIZE);
@@ -2328,7 +2326,6 @@ check_redzone(const void *vaddr, const struct 
intel_engine_cs *engine)
if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
return;
 
-   vaddr += LRC_HEADER_PAGES * PAGE_SIZE;
vaddr += engine->context_size;
 
if (memchr_inv(vaddr, POISON_INUSE, I915_GTT_PAGE_SIZE))
@@ -3995,12 +3992,6 @@ populate_lr_context(struct intel_context *ce,
set_redzone(vaddr, engine);
 
if (engine->default_state) {
-   /*
-* We only want to copy over the template context state;
-* skipping over the headers reserved for GuC communication,
-* leaving those as zero.
-*/
-   const unsigned long start = LRC_HEADER_PAGES * PAGE_SIZE;
void *defaults;
 
defaults = i915_gem_object_pin_map(engine->default_state,
@@ -4010,7 +4001,7 @@ populate_lr_context(struct intel_context *ce,
goto err_unpin_ctx;
}
 
-   memcpy(vaddr + start, defaults + start, engine->context_size);
+   memcpy(vaddr, defaults, engine->context_size);
i915_gem_object_unpin_map(engine->default_state);
inhibit = false;
}
@@ -4025,9 +4016,7 @@ populate_lr_context(struct intel_context *ce,
 
ret = 0;
 err_unpin_ctx:
-   __i915_gem_object_flush_map(ctx_obj,
-   LRC_HEADER_PAGES * PAGE_SIZE,
-   engine->context_size);
+   __i915_gem_object_flush_map(ctx_obj, 0, engine->context_size);
i915_gem_object_unpin_map(ctx_obj);
return ret;
 }
@@ -4044,11 +4033,6 @@ static int __execlists_context_alloc(struct 
intel_context *ce,
GEM_BUG_ON(ce->state);
context_size = round_up(engine->context_size, I915_GTT_PAGE_SIZE);
 
-   /*
-* Before the actual start of the context image, we insert a few pages
-* for our own use and for sharing with the GuC.
-*/
-   context_size += LRC_HEADER_PAGES * PAGE_SIZE;
if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
context_size += I915_GTT_PAGE_SIZE; /* for redzone */
 
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h 
b/drivers/gpu/drm/i915/gt/intel_lrc.h
index faa2d56c279b..04511d8ebdc1 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.h
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.h
@@ -86,31 +86,12 @@ int intel_execlists_submission_setup(struct intel_engine_cs 
*engine);
 int intel_execlists_submission_init(struct intel_engine_cs *engine);
 
 /* Logical Ring Contexts */
-
-/*
- * We allocate a header at the start of the context image for our own
- * use, therefore the actual location of the logical state is offset
- * from the start of the VMA. The layout is
- *
- * | [guc]  | [hwsp] [logical state] |
- * |<- our header ->|<- context image  ->|
- *
- */
-/* The first page is used for sharing data with the GuC */
-#define LRC_GUCSHR_PN  (0)
-#define LRC_GUCSHR_SZ  (1)
 /* At the start of the context image is its per-process HWS page */
-#define LRC_PPHWSP_PN  (LRC_GUCSHR_PN + LRC_GUCSHR_SZ)
+#define LRC_PPHWSP_PN  (0)
 #define LRC_PPHWSP_SZ  (1)
-/* Finally we have the logical