| Message ID | 20181018183024.31156-1-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/2] drm/i915/guc: Limit number of scratch registers used for H2G | expand |
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@intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > 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@intel.com> 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 >
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. * * +-----------+---------+---------+---------+ * | MMIO[0] | MMIO[1] | ... | MMIO[n] | @@ -633,6 +634,8 @@ struct guc_shared_ctx_data { * field. */ +#define GUC_MMIO_MSG_LEN 8 + #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
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@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_guc.c | 3 ++- drivers/gpu/drm/i915/intel_guc_fwif.h | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-)