On Wed, Jan 13, 2016 at 12:28:17PM +0000, Arun Siluvery wrote:
> Some of the HW registers are privileged and cannot be written to from a
> non-privileged batch buffers coming from userspace unless they are on whitelist.
> Userspace need write access to them to implement preemption related WA.
You need to be clear that this is the hw whitelist and not our sw
whitelist.
> 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).
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> Signed-off-by: Arun Siluvery <arun.siluvery@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 9 ++++++++-
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> drivers/gpu/drm/i915/intel_ringbuffer.c | 17 +++++++++++++++++
> 3 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 104bd18..660afe1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1653,11 +1653,18 @@ struct i915_wa_reg {
> u32 mask;
> };
>
> -#define I915_MAX_WA_REGS 16
> +/*
> + * RING_MAX_NONPRIV_SLOTS is per-engine but at this point we are only
> + * allowing it for RCS as we don't foresee any requirement of having
> + * a whitelist for other engines. When it is really required for
> + * other engines then the limit need to be increased.
> + */
> +#define I915_MAX_WA_REGS (16 + RING_MAX_NONPRIV_SLOTS)
>
> struct i915_workarounds {
> struct i915_wa_reg reg[I915_MAX_WA_REGS];
> u32 count;
> + u32 hw_whitelist_index[I915_NUM_RINGS];
This is a counter only to be used whilst constructing the reg list. It
also implies that this list of wa_reg is now engine specific. Hmm, looks
like it always has been, just been relying on RCS running first.
I would fix that up first.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
