On 13/01/2016 19:01, Chris Wilson wrote:
On Wed, Jan 13, 2016 at 03:38:15PM +0000, Arun Siluvery wrote:
Some of the HW registers are privileged and cannot be written to from
non-privileged batch buffers coming from userspace unless they are added to
the HW whitelist. This whitelist is maintained by HW and it is different from
SW whitelist. Userspace need write access to them to implement preemption
related WA.
The reason for using this approach is, the register bits that control
preemption granularity at the HW level are not context save/restored; so even
if we set these bits always in kernel they are going to change once the
context is switched out. We can consider making them non-privileged by
default but these registers also contain other chicken bits which should not
be allowed to be modified.
In the later revisions controlling bits are save/restored at context level but
in the existing revisions these are exported via other debug registers and
should be on the whitelist. This patch adds changes to provide HW with a list
of registers to be whitelisted. HW checks this list during execution and
provides access accordingly.
HW imposes a limit on the number of registers on whitelist and it is
per-engine. At this point we are only enabling whitelist for RCS and we don't
foresee any requirement for other engines.
The registers to be whitelisted are added using generic workaround list
mechanism, even these are only enablers for userspace workarounds. But by
sharing this mechanism we get some test assets without additional cost (Mika).
v2: rebase
v3: parameterize RING_FORCE_TO_NONPRIV() as _MMIO() should be limited to
i915_reg.h (Ville), drop inline for wa_ring_whitelist_reg (Mika).
v4: Clarify that this is HW whitelist and different from the one maintained in
driver. This list is engine specific but it gets initialized along with other
WA which is RCS specific thing, so make it clear that we are not doing any
cross engine setup during initialization (Chris).
Those name work much better for me, so thanks for clearing them up and
allaying my fears.
Would it not also make sense to expose hw_whitelist_count[] in
i915_wa_registers (debugfs)?
It is already is part of i915_wa_registers, each HW whitelist entry is
just another entry in i915_workarounds. Mika suggested to add this using
workaround list mechanism so that we get this without additional cost.
+static int wa_ring_whitelist_reg(struct intel_engine_cs *ring,
+ i915_reg_t reg_addr)
+{
+ struct drm_i915_private *dev_priv = ring->dev->dev_private;
+ struct i915_workarounds *wa = &dev_priv->workarounds;
+ const uint32_t index = wa->hw_whitelist_count[ring->id];
+
+ if (WARN_ON(index >= RING_MAX_NONPRIV_SLOTS))
+ return -EINVAL;
+
+ WA_WRITE(RING_FORCE_TO_NONPRIV(ring->mmio_base, index), reg_addr.reg);
WA_WRITE(RING_FORCE_TO_NONPRIV(ring->mmio_base, index),
i915_mmio_reg_offset(reg_addr));
And just call it reg. (reg_addr would imply that you applied the mmio
offset, i.e were about to call ioread32(reg_addr)).
the thought of using i915_mmio_reg_offset() came to me after sending v3
but thought no one would notice :)
Do you want me to change this and send again?
- WA_WRITE(RING_FORCE_TO_NONPRIV(ring->mmio_base, index),
reg_addr.reg);
+ WA_WRITE(RING_FORCE_TO_NONPRIV(ring->mmio_base, index),
+ i915_mmio_reg_offset(reg));
regards
Arun
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
