The attached version still does not explain that the WOPCM_TOP is to tell the GuC not to use that space. The extra information does not aid anybody as the information is used internally within the GuC. But, I have not actual objection to the patch. Peter. -----Original Message----- From: Gordon, David S Sent: Thursday, May 5, 2016 2:41 PM To: Antoine, Peter <peter.antoine@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx> Subject: Re: [PATCH v3] drm/i915: resize the GuC WOPCM for rc6 On 26/04/2016 10:11, Peter Antoine wrote: > This patch resizes the GuC WOPCM to so that the GuC and the RC6 memory > spaces do not overlap. > > Issue: https://jira01.devtools.intel.com/browse/VIZ-6638 > Signed-off-by: Peter Antoine <peter.antoine@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_guc_reg.h | 5 +++-- > drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++++- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h > b/drivers/gpu/drm/i915/i915_guc_reg.h > index 80786d9..6e01238 100644 > --- a/drivers/gpu/drm/i915/i915_guc_reg.h > +++ b/drivers/gpu/drm/i915/i915_guc_reg.h > @@ -68,10 +68,11 @@ > #define GUC_MAX_IDLE_COUNT _MMIO(0xC3E4) > > #define GUC_WOPCM_SIZE _MMIO(0xc050) > -#define GUC_WOPCM_SIZE_VALUE (0x80 << 12) /* 512KB */ > +#define GUC_WOPCM_SIZE_VALUE (0x80 << 12) /* 512KB */ > +#define BXT_GUC_WOPCM_SIZE_VALUE (0x70 << 12) /* 448KB */ > > /* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */ > -#define GUC_WOPCM_TOP (GUC_WOPCM_SIZE_VALUE) > +#define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */ > > #define GEN8_GT_PM_CONFIG _MMIO(0x138140) > #define GEN9LP_GT_PM_CONFIG _MMIO(0x138140) > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index fc3ff68..38fb321 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -312,7 +312,11 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv) > intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); > > /* init WOPCM */ > - I915_WRITE(GUC_WOPCM_SIZE, GUC_WOPCM_SIZE_VALUE); > + if (IS_BROXTON(dev)) > + I915_WRITE(GUC_WOPCM_SIZE, BXT_GUC_WOPCM_SIZE_VALUE); > + else > + I915_WRITE(GUC_WOPCM_SIZE, GUC_WOPCM_SIZE_VALUE); > + > I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE); > > /* Enable MIA caching. GuC clock gating is disabled. */ So, this gives the right result, but doesn't really show or explain why we have different values, or how the values are arrived at; they're just more magic numbers. Also, in the loader there's a check on the firmware size that uses different values. So I'd rather prefer the unified approach in the attached version ... .Dave. _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
