[Intel-gfx] [PATCH] drm/i915/guc: fix mmio whitelist mmio_start offset and add reminder

[Michał Winiarski]

On Thu, Dec 29, 2016 at 05:32:47PM -0800, daniele.ceraolospurio at intel.com wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> 
> 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 at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> ---
>  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ł

>  		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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx