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.anto...@intel.com>; intel-gfx@lists.freedesktop.org
Cc: Vivi, Rodrigo <rodrigo.v...@intel.com>
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.anto...@intel.com>
> ---
>   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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to