On Wed, Jan 13, 2016 at 10:06:26AM +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.
>
> 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
>
> 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 | 18 ++++++++++++++++++
> 3 files changed, 29 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];
> };
>
> struct i915_virtual_gpu {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0a98889..6668bb0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1635,6 +1635,9 @@ enum skl_disp_power_wells {
> #define RING_WAIT (1<<11) /* gen3+, PRBx_CTL */
> #define RING_WAIT_SEMAPHORE (1<<10) /* gen6+ */
>
> +#define RING_FORCE_TO_NONPRIV(base) ((base)+0x4D0)
> +#define RING_MAX_NONPRIV_SLOTS 12
> +
> #define GEN7_TLB_RD_ADDR _MMIO(0x4700)
>
> #if 0
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 4060acf..354da81 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -787,6 +787,23 @@ static int wa_add(struct drm_i915_private *dev_priv,
>
> #define WA_WRITE(addr, val) WA_REG(addr, 0xffffffff, val)
>
> +static inline 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_index[ring->id];
> +
> + if (WARN_ON(index >= RING_MAX_NONPRIV_SLOTS))
> + return -EINVAL;
> +
> + WA_WRITE(_MMIO(RING_FORCE_TO_NONPRIV(ring->mmio_base) + index * 4),
> + reg_addr.reg);
No _MMIO junk outside of i915_reg.h please. Just parametrize the reg define
properly.
> + wa->hw_whitelist_index[ring->id]++;
> +
> + return 0;
> +}
> +
> static int gen8_init_workarounds(struct intel_engine_cs *ring)
> {
> struct drm_device *dev = ring->dev;
> @@ -1115,6 +1132,7 @@ int init_workarounds_ring(struct intel_engine_cs *ring)
> WARN_ON(ring->id != RCS);
>
> dev_priv->workarounds.count = 0;
> + dev_priv->workarounds.hw_whitelist_index[ring->id] = 0;
>
> if (IS_BROADWELL(dev))
> return bdw_init_workarounds(ring);
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
