Chris Wilson <ch...@chris-wilson.co.uk> writes:
> For each platform, we have a few registers that rewritten with multiple
> values -- they are not part of a sequence, just different parts of a
> masked register set at different times (e.g. platform and gen
> workarounds). Consolidate these into a single register write to keep the
> table compact.
>
> While adjusting the construction of the wa table, make it non fatal so
> that the driver still loads but keeping the warning and extra details
> for inspection.
>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Oscar Mateo <oscar.ma...@intel.com>
> Cc: Mika Kuoppala <mika.kuopp...@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 25 ++--------
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_workarounds.c | 63 +++++++++++++++++-------
> 3 files changed, 52 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index c600279d3db5..f78895ffab9b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3378,28 +3378,13 @@ static int i915_shared_dplls_info(struct seq_file *m,
> void *unused)
>
> static int i915_wa_registers(struct seq_file *m, void *unused)
> {
> - struct drm_i915_private *dev_priv = node_to_i915(m->private);
> - struct i915_workarounds *workarounds = &dev_priv->workarounds;
> + struct i915_workarounds *wa = &node_to_i915(m->private)->workarounds;
> int i;
>
> - intel_runtime_pm_get(dev_priv);
> -
> - seq_printf(m, "Workarounds applied: %d\n", workarounds->count);
> - for (i = 0; i < workarounds->count; ++i) {
> - i915_reg_t addr;
> - u32 mask, value, read;
> - bool ok;
> -
> - addr = workarounds->reg[i].addr;
> - mask = workarounds->reg[i].mask;
> - value = workarounds->reg[i].value;
> - read = I915_READ(addr);
> - ok = (value & mask) == (read & mask);
> - seq_printf(m, "0x%X: 0x%08X, mask: 0x%08X, read: 0x%08x,
> status: %s\n",
> - i915_mmio_reg_offset(addr), value, mask, read, ok ?
> "OK" : "FAIL");
> - }
> -
> - intel_runtime_pm_put(dev_priv);
> + seq_printf(m, "Workarounds applied: %d\n", wa->count);
> + for (i = 0; i < wa->count; ++i)
> + seq_printf(m, "0x%X: 0x%08X, mask: 0x%08X\n",
> + wa->reg[i].addr, wa->reg[i].value, wa->reg[i].mask);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2c12de678e32..91c389622217 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1308,7 +1308,7 @@ struct i915_frontbuffer_tracking {
> };
>
> struct i915_wa_reg {
> - i915_reg_t addr;
> + u32 addr;
> u32 value;
> /* bitmask representing WA bits */
> u32 mask;
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c
> b/drivers/gpu/drm/i915/intel_workarounds.c
> index 24b929ce3341..f8bb32e974f6 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -48,29 +48,58 @@
> * - Public functions to init or apply the given workaround type.
> */
>
> -static int wa_add(struct drm_i915_private *dev_priv,
> - i915_reg_t addr,
> - const u32 mask, const u32 val)
> +static void wa_add(struct drm_i915_private *i915,
> + i915_reg_t reg, const u32 mask, const u32 val)
> {
> - const unsigned int idx = dev_priv->workarounds.count;
> + struct i915_workarounds *wa = &i915->workarounds;
> + unsigned int start = 0, end = wa->count;
> + unsigned int addr = i915_mmio_reg_offset(reg);
> + struct i915_wa_reg *r;
> +
> + while (start < end) {
> + unsigned int mid = start + (end - start) / 2;
> +
> + if (wa->reg[mid].addr < addr) {
> + start = mid + 1;
> + } else if (wa->reg[mid].addr > addr) {
> + end = mid;
> + } else {
> + r = &wa->reg[mid];
> +
> + if ((mask & ~r->mask) == 0) {
> + DRM_ERROR("Discarding overwritten w/a for reg
> %04x (mask: %08x, value: %08x)\n",
> + addr, r->mask, r->value);
> +
> + r->value &= ~mask;
> + }
> +
> + r->value |= val;
> + r->mask |= mask;
> + return;
> + }
> + }
>
> - if (WARN_ON(idx >= I915_MAX_WA_REGS))
> - return -ENOSPC;
> + if (WARN_ON_ONCE(wa->count >= I915_MAX_WA_REGS)) {
> + DRM_ERROR("Dropping w/a for reg %04x (mask: %08x, value:
> %08x)\n",
> + addr, mask, val);
> + return;
> + }
>
> - dev_priv->workarounds.reg[idx].addr = addr;
> - dev_priv->workarounds.reg[idx].value = val;
> - dev_priv->workarounds.reg[idx].mask = mask;
> + r = &wa->reg[wa->count++];
> + r->addr = addr;
> + r->value = val;
> + r->mask = mask;
>
> - dev_priv->workarounds.count++;
> + while (r-- > wa->reg) {
> + GEM_BUG_ON(r[0].addr == r[1].addr);
> + if (r[1].addr > r[0].addr)
> + break;
>
> - return 0;
> + swap(r[1], r[0]);
> + }
> }
>
> -#define WA_REG(addr, mask, val) do { \
> - const int r = wa_add(dev_priv, (addr), (mask), (val)); \
> - if (r) \
> - return r; \
> - } while (0)
> +#define WA_REG(addr, mask, val) wa_add(dev_priv, (addr), (mask), (val))
On previous thread, I noted that this makes the wa init chain
not needing return values.
But cleaning that up can be a followup.
Reviewed-by: Mika Kuoppala <mika.kuopp...@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
