[Intel-gfx] [PATCH 2/6] drm/i915/gt: Lock intel_engine_apply_whitelist with uncore->lock

[Chris Wilson]

Quoting Umesh Nerlige Ramappa (2020-10-10 01:21:01)
> Refactor intel_engine_apply_whitelist into locked and unlocked versions
> so that a caller who already has the lock can apply whitelist.
> 
> v2: Fix sparse warning
> 
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 44 +++++++++++++++------
>  1 file changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 6c580d0d9ea8..864194aa6eef 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1295,7 +1295,8 @@ void intel_gt_init_workarounds(struct drm_i915_private *i915)
>  }
>  
>  static enum forcewake_domains
> -wal_get_fw_for_rmw(struct intel_uncore *uncore, const struct i915_wa_list *wal)
> +wal_get_fw(struct intel_uncore *uncore, const struct i915_wa_list *wal,
> +          unsigned int op)
>  {
>         enum forcewake_domains fw = 0;
>         struct i915_wa *wa;
> @@ -1304,8 +1305,7 @@ wal_get_fw_for_rmw(struct intel_uncore *uncore, const struct i915_wa_list *wal)
>         for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
>                 fw |= intel_uncore_forcewake_for_reg(uncore,
>                                                      wa->reg,
> -                                                    FW_REG_READ |
> -                                                    FW_REG_WRITE);
> +                                                    op);
>  
>         return fw;
>  }
> @@ -1335,7 +1335,7 @@ wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal)
>         if (!wal->count)
>                 return;
>  
> -       fw = wal_get_fw_for_rmw(uncore, wal);
> +       fw = wal_get_fw(uncore, wal, FW_REG_READ | FW_REG_WRITE);
>  
>         spin_lock_irqsave(&uncore->lock, flags);
>         intel_uncore_forcewake_get__locked(uncore, fw);
> @@ -1645,27 +1645,45 @@ void intel_engine_init_whitelist(struct intel_engine_cs *engine)
>         wa_init_finish(w);
>  }
>  
> -void intel_engine_apply_whitelist(struct intel_engine_cs *engine)
> +static void intel_engine_apply_whitelist_locked(struct intel_engine_cs *engine)

Not too bad, but since it remains static, we could drop the prefix (and
suffix):
	__engine_apply_whitelist(engine, wal)

>  {
>         const struct i915_wa_list *wal = &engine->whitelist;
>         struct intel_uncore *uncore = engine->uncore;
>         const u32 base = engine->mmio_base;
>         struct i915_wa *wa;
>         unsigned int i;
> +       enum forcewake_domains fw;

(Christmas tree)

>  
> -       if (!wal->count)
> -               return;
> +       lockdep_assert_held(&uncore->lock);
> +
> +       fw = wal_get_fw(uncore, wal, FW_REG_WRITE);
> +       intel_uncore_forcewake_get__locked(uncore, fw);
>  
>         for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
> -               intel_uncore_write(uncore,
> -                                  RING_FORCE_TO_NONPRIV(base, i),
> -                                  i915_mmio_reg_offset(wa->reg));
> +               intel_uncore_write_fw(uncore,
> +                                     RING_FORCE_TO_NONPRIV(base, i),
> +                                     i915_mmio_reg_offset(wa->reg));
>  
>         /* And clear the rest just in case of garbage */
>         for (; i < RING_MAX_NONPRIV_SLOTS; i++)
> -               intel_uncore_write(uncore,
> -                                  RING_FORCE_TO_NONPRIV(base, i),
> -                                  i915_mmio_reg_offset(RING_NOPID(base)));
> +               intel_uncore_write_fw(uncore,
> +                                     RING_FORCE_TO_NONPRIV(base, i),
> +                                     i915_mmio_reg_offset(RING_NOPID(base)));
> +
> +       intel_uncore_forcewake_put__locked(uncore, fw);
> +}
> +
> +void intel_engine_apply_whitelist(struct intel_engine_cs *engine)
> +{
> +       unsigned long flags;
> +       const struct i915_wa_list *wal = &engine->whitelist;

Longest to shortest variable declaration (if no other reason prevents
that).

Reviewed-by: Chris Wilson <chris.p.wilson at intel.com>
-Chris
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.