On 1/2/2017 4:18 AM, Michał Winiarski wrote:
On Thu, Dec 29, 2016 at 05:32:47PM -0800, daniele.ceraolospurio@xxxxxxxxx wrote:
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
The mmio_start offset for the whitelist is the first FORCE_TO_NONPRIV
register the GuC can use to restore the provided whitelist when an
engine reset via GuC (which we still don't support) is triggered.
We're currently adding the mmio_base of the engine to the absolute
address of the RCS version of the register, which results in the wrong
offset. Fix it by using the definition we already have instead of
re-defining it in the GuC FW header.
Also add a comment to avoid future issues with FORCE_TO_NONPRIV
registers, which are also used by the workaround framework.
Compile tested only as my SKL died early this week.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 9 +++++++--
drivers/gpu/drm/i915/intel_guc_fwif.h | 1 -
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 30e012b..7a1c6a0 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1336,9 +1336,14 @@ static void guc_addon_create(struct intel_guc *guc)
for_each_engine(engine, dev_priv, id) {
reg_state->mmio_white_list[engine->guc_id].mmio_start =
- engine->mmio_base + GUC_MMIO_WHITE_LIST_START;
+ i915_mmio_reg_offset(RING_FORCE_TO_NONPRIV(engine->mmio_base, 0));
- /* Nothing to be saved or restored for now. */
+ /*
+ * Nothing to be saved or restored for now.
+ * XXX: when adding registers to the whitelist, make sure to not
+ * conflict with what the workaround framework is doing with
+ * the FORCE_TO_NONPRIV registers.
+ */
Is this what will end up happening with GuC engine reset?
Shouldn't we just use the workaround framework to populate GuC whitelist instead
of maintaining separate one? (in which case, the comment is slightly misleading)
-Michał
I'm not sure which way we'll go with GuC engine as that will be an
implementation detail, because AFAIK writing the registers manually using the
workaround framework as we do now (with count=0 here) and moving the handling of
post-reset restore of registers to GuC should functionally be identical. I agree
with you that if we decide to use the GuC to do the restore we should use the
workaround framework to populate this list, but since I'm not sure which way
we're going I wanted to keep the comment generic. Does something like this sound
better to you
/* Note: if the GuC whitelist management is enabled the values
* should be filled using the workaround framework to avoid
* inconsistencies with the handling of FORCE_TO_NONPRIV
* registers
*/
I'm open to suggestions if you still feel that this is not clear enough ;).
Thanks,
Daniele
reg_state->mmio_white_list[engine->guc_id].count = 0;
}
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 3202b32..2841d59 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -371,7 +371,6 @@ struct guc_policies {
#define GUC_REGSET_SAVE_CURRENT_VALUE 0x10
#define GUC_REGSET_MAX_REGISTERS 25
-#define GUC_MMIO_WHITE_LIST_START 0x24d0
#define GUC_MMIO_WHITE_LIST_MAX 12
#define GUC_S3_SAVE_SPACE_PAGES 10
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
