On 09/10/17 15:35, Michel Thierry wrote:
On 10/9/2017 11:41 AM, Daniele Ceraolo Spurio wrote:
On 09/10/17 07:52, Michał Winiarski wrote:
We were using first page of kernel context render state for sharing data
with GuC. While it's justified by the fact that those pages are not used
(note, GuC still enforces this layout and refuses to work if we remove
the extra page in front), it's also confusing (why are we using this
particular page?). Let's allocate a separate object instead.
Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Jeff McGee <jeff.mcgee@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
+Michel (engine and watchdog reset with GuC use the shared page)
---
drivers/gpu/drm/i915/i915_guc_submission.c | 36
+++++++++++++++++++++++++++++-
drivers/gpu/drm/i915/intel_guc.c | 8 ++-----
drivers/gpu/drm/i915/intel_guc.h | 2 ++
3 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 8983d53af229..30f026566001 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -437,6 +437,33 @@ static void guc_stage_desc_fini(struct intel_guc
*guc,
memset(desc, 0, sizeof(*desc));
}
+static int guc_shared_data_create(struct intel_guc *guc)
+{
+ struct i915_vma *vma;
+ void *vaddr;
+
+ vma = intel_guc_allocate_vma(guc, PAGE_SIZE);
+ if (IS_ERR(vma))
+ return PTR_ERR(vma);
+
+ vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
+ if (IS_ERR(vaddr)) {
+ i915_vma_unpin_and_release(&vma);
+ return PTR_ERR(vaddr);
+ }
+
+ guc->shared_data = vma;
+ guc->shared_data_vaddr = vaddr;
Hi,
Allocating the shared_data until this point (i915_guc_submission_init)
will be too late for GuC's watchdog.
GuC watchdog happens without i915 knowledge, so we have to pass this
shared_data_offset during guc_params_init (in params[9] for the curious)
instead of a h2g command; and the GuC parameters block has this note:
"These parameters are read by the firmware on startup and cannot be
changed thereafter".
Michał, if you plan to send another version of this, could you move it
to guc_params_init? It isn't a big issue, I can just move it when we
have an open source user and can upstream GuC watchdog.
Thanks,
-Michel
Ignore my previous reply, this is already being allocated before
guc_params_init as it is.
Reviewed-by: Michel Thierry <michel.thierry@xxxxxxxxx>
I've noticed that this is now the 3rd place where we allocate and
immediately pin a vma for guc (the other 2 being
guc_stage_desc_pool_create and ctch_init). Maybe we can move the 2
operations to a more common helper (and do the same for the cleanup),
but it's probably better to do it after all the ongoing GuC re-org has
settled down. In the meantime, this patch is:
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Daniele
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
