Re: [PATCH 1/4] drm/i915: Support flags in whitlist WAs

Tvrtko Ursulin
2019-06-14T06:48:58Z

On 14/06/2019 01:28, Robert M. Fosha wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Newer hardware adds flags to the whitelist work-around register. These
> allow per access direction privileges and ranges.
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Signed-off-by: Robert M. Fosha <robert.m.fosha@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 9 ++++++++-
> drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 165b0a45e009..ae82340fff45 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1012,7 +1012,7 @@ bool intel_gt_verify_workarounds(struct drm_i915_private *i915,
> }
>
> static void
> -whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
> +whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
> {
> struct i915_wa wa = {
> .reg = reg
> @@ -1021,9 +1021,16 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
> if (GEM_DEBUG_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
> return;
>
> + wa.reg.reg |= flags;
> _wa_add(wal, &wa);
> }
>
> +static void
> +whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
> +{
> + whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_RW);
> +}
> +
> static void gen9_whitelist_build(struct i915_wa_list *w)
> {
> /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index edf9f93934a1..10fea5ab3fc3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2513,6 +2513,13 @@ enum i915_power_well_id {
> #define RING_WAIT_SEMAPHORE (1 << 10) /* gen6+ */
>
> #define RING_FORCE_TO_NONPRIV(base, i) _MMIO(((base) + 0x4D0) + (i) * 4)
> +#define RING_FORCE_TO_NONPRIV_RW (0 << 28) /* CFL+ & Gen11+ */

Shouldn't this comment be against the RD and WR flags, while the above
is the legacy one?

> +#define RING_FORCE_TO_NONPRIV_RD (1 << 28)
> +#define RING_FORCE_TO_NONPRIV_WR (2 << 28)
> +#define RING_FORCE_TO_NONPRIV_RANGE_1 (0 << 0) /* CFL+ & Gen11+ */
> +#define RING_FORCE_TO_NONPRIV_RANGE_4 (1 << 0)
> +#define RING_FORCE_TO_NONPRIV_RANGE_16 (2 << 0)
> +#define RING_FORCE_TO_NONPRIV_RANGE_64 (3 << 0)
> #define RING_MAX_NONPRIV_SLOTS 12
>
> #define GEN7_TLB_RD_ADDR _MMIO(0x4700)
>

Regards,

Tvrtko