On 7/12/2017 12:45 PM, Chris Wilson wrote:
Quoting Michel Thierry (2017-07-12 20:30:31)
Not only the context image consist of two parts (the PPHWSP, and the
logical context state), but we also allocate a header at the start of
for sharing data with GuC. Thus every lrc looks like this:
| [guc] | [hwsp] [logical state] |
|<- our header ->|<- context image ->|
So far, we have oversimplified whenever we use each of these parts of the
context, just because the GuC header happens to be in page 0, and the
(PP)HWSP is in page 1. But this had led to using the same define for more
than one meaning (as a page index in the lrc and as 1 page).
This patch adds defines for the GuC shared page, the PPHWSP page and the
start of the logical state. It also updated the places where the old
define was being used. Since we are not changing the size (or format) of
the context, there are no functional changes.
Suggested-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Michel Thierry <michel.thierry@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Cc: intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx
---
drivers/gpu/drm/i915/gvt/scheduler.c | 4 ++--
drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++--
drivers/gpu/drm/i915/intel_lrc.c | 11 +++++++----
drivers/gpu/drm/i915/intel_lrc.h | 25 ++++++++++++++++++++++---
4 files changed, 33 insertions(+), 11 deletions(-)
...
@@ -1695,7 +1695,7 @@ logical_ring_default_irqs(struct intel_engine_cs *engine)
static int
lrc_setup_hws(struct intel_engine_cs *engine, struct i915_vma *vma)
{
- const int hws_offset = LRC_PPHWSP_PN * PAGE_SIZE;
+ const int hws_offset = LRC_HEADER_PAGES * PAGE_SIZE;
This should be PPHWSP_PN since we explicitly want the pointer tot he
pphwsp.
Oh right, this was the only place where it was really correct.
I was about to resend it with this change, but then realised it just
means the lrc_setup_hws chunk of the patch disappears... let me know if
I should still resend it.
Thanks,
void *hws;
/* The HWSP is part of the default context object in LRC mode. */
@@ -2015,8 +2015,11 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
context_size = round_up(engine->context_size, I915_GTT_PAGE_SIZE);
- /* One extra page as the sharing data between driver and GuC */
- context_size += PAGE_SIZE * LRC_PPHWSP_PN;
+ /*
+ * 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;
Good riddance, that LRC_PPHWSP_PN has left me quite confused for some
time.
With the change to lrc_setup_hws,
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
