On 18/10/18 11:30, Michal Wajdeczko wrote:
We wrongly assumed that GuC is only using last scratch register
for G2H messages, but in fact it is also using register [14] to
report sleep state status. Remove that register from our H2G
send registers pool.
v2: No message from host to GuC uses more than 8 registers and
the GuC FW itself uses an 8-element array to store the H2G message,
so we may reduce our send array to just 8 registers (Daniele)
v3: use explicit define
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/intel_guc.c | 3 ++-
drivers/gpu/drm/i915/intel_guc_fwif.h | 5 ++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 4c61eb9..9b00cdf 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -50,7 +50,8 @@ void intel_guc_init_send_regs(struct intel_guc *guc)
unsigned int i;
guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
- guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
+ guc->send_regs.count = GUC_MMIO_MSG_LEN;
+ BUILD_BUG_ON(GUC_MMIO_MSG_LEN > SOFT_SCRATCH_COUNT);
for (i = 0; i < guc->send_regs.count; i++) {
fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index d1bbaba..cc997ef 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -601,7 +601,8 @@ struct guc_shared_ctx_data {
* registers, where first register holds data treated as message header,
* and other registers are used to hold message payload.
*
- * For Gen9+, GuC uses software scratch registers 0xC180-0xC1B8
+ * For Gen9+, GuC uses software scratch registers 0xC180-0xC1B8, but
+ * the GuC FW itself uses an 8-element array to store the H2G message.
I think our justification here should be that no commands takes more
than 8 dwords more than what GuC FW does internally (which can reinforce
the point but not make it IMO).
*
* +-----------+---------+---------+---------+
* | MMIO[0] | MMIO[1] | ... | MMIO[n] |
@@ -633,6 +634,8 @@ struct guc_shared_ctx_data {
* field.
*/
+#define GUC_MMIO_MSG_LEN 8
+
I'd prefer GUC_MAX_MMIO_MSG_LEN, but I can live with this.
With the improved comment:
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Daniele
#define INTEL_GUC_MSG_TYPE_SHIFT 28
#define INTEL_GUC_MSG_TYPE_MASK (0xF << INTEL_GUC_MSG_TYPE_SHIFT)
#define INTEL_GUC_MSG_DATA_SHIFT 16
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
