[Intel-gfx] [PATCH v2 2/5] drm/i915/uc: Move uC WOPCM setup in uc_init_hw

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-07-31 00:07:40)
> The register we write are not WOPCM regs but uC ones related to how
> GuC and HuC are going to use the WOPCM, so it makes logical sense
> for them to be programmed as part of uc_init_hw. The WOPCM map on the
> other side is not uC-specific (although that is our main use-case), so
> keep that separate.
> 
> v2: move write_and_verify to uncore, fix log, re-use err_out tag,
>     add intel_wopcm_guc_base, fix log
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 47 ++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h       |  3 +-
>  drivers/gpu/drm/i915/i915_gem.c       |  8 +---
>  drivers/gpu/drm/i915/intel_uncore.h   | 12 +++++
>  drivers/gpu/drm/i915/intel_wopcm.c    | 68 ---------------------------
>  drivers/gpu/drm/i915/intel_wopcm.h    | 18 +++++--
>  6 files changed, 76 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 657fdcb70d00..7794a6a1f932 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -399,6 +399,49 @@ void intel_uc_sanitize(struct intel_uc *uc)
>         __uc_sanitize(uc);
>  }
>  
> +/* Initialize and verify the uC regs related to uC positioning in WOPCM */
> +static int uc_init_wopcm(struct intel_uc *uc)
> +{
> +       struct intel_gt *gt = uc_to_gt(uc);
> +       struct intel_uncore *uncore = gt->uncore;
> +       u32 base = intel_wopcm_guc_base(&gt->i915->wopcm);
> +       u32 size = intel_wopcm_guc_size(&gt->i915->wopcm);
> +       u32 huc_agent = intel_uc_is_using_huc(uc) ? HUC_LOADING_AGENT_GUC : 0;
> +       u32 mask;
> +       int err;
> +
> +       GEM_BUG_ON(!intel_uc_is_using_guc(uc));
> +       GEM_BUG_ON(!(base & GUC_WOPCM_OFFSET_MASK));
> +       GEM_BUG_ON(base & ~GUC_WOPCM_OFFSET_MASK);
> +       GEM_BUG_ON(!(size & GUC_WOPCM_SIZE_MASK));
> +       GEM_BUG_ON(size & ~GUC_WOPCM_SIZE_MASK);

Worth a GEM_BUG_ON(base + size < base) ?

> +
> +       mask = GUC_WOPCM_SIZE_MASK | GUC_WOPCM_SIZE_LOCKED;
> +       err = intel_uncore_write_and_verify(uncore, GUC_WOPCM_SIZE, size, mask,
> +                                           size | GUC_WOPCM_SIZE_LOCKED);
> +       if (err)
> +               goto err_out;
> +
> +       mask = GUC_WOPCM_OFFSET_MASK | GUC_WOPCM_OFFSET_VALID | huc_agent;
> +       err = intel_uncore_write_and_verify(uncore, DMA_GUC_WOPCM_OFFSET,
> +                                           base | huc_agent, mask,
> +                                           base | huc_agent |
> +                                           GUC_WOPCM_OFFSET_VALID);
> +       if (err)
> +               goto err_out;
> +
> +       return 0;
> +
> +err_out:
> +       DRM_ERROR("Failed to init uC WOPCM registers:\n");
> +       DRM_ERROR("DMA_GUC_WOPCM_OFFSET=%#x\n",
> +                 intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET));
> +       DRM_ERROR("GUC_WOPCM_SIZE=%#x\n",
> +                 intel_uncore_read(uncore, GUC_WOPCM_SIZE));
> +
> +       return err;
> +}

Fair enough,
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris