On 12/10/17 13:35, Michel Thierry wrote:
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 spoke too soon (and sorry for all the spam), the kernel_context is now
redundant code and should be removed from the suspend & resume functions:
diff --git a/drivers/gpu/drm/i915/intel_guc.c
b/drivers/gpu/drm/i915/intel_guc.c
index 5cd9bc53e021..47c74ef0bd23 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -176,7 +176,6 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32
rsa_offset)
int intel_guc_suspend(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
- struct i915_gem_context *ctx;
u32 data[3];
if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
@@ -184,8 +183,6 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
gen9_disable_guc_interrupts(dev_priv);
- ctx = dev_priv->kernel_context;
-
data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
/* any value greater than GUC_POWER_D0 */
data[1] = GUC_POWER_D1;
@@ -225,7 +222,6 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
int intel_guc_resume(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
- struct i915_gem_context *ctx;
u32 data[3];
if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
@@ -234,8 +230,6 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
if (i915_modparams.guc_log_level >= 0)
gen9_enable_guc_interrupts(dev_priv);
- ctx = dev_priv->kernel_context;
-
data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
data[1] = GUC_POWER_D0;
data[2] = guc_ggtt_offset(guc->shared_data);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
