[Intel-gfx] [PATCH v1 1/2] drm/i915/icl: Mind the SFC units when resetting VD or VEBox engines
Chris Wilson
chris at chris-wilson.co.uk
Wed Oct 31 14:27:06 UTC 2018
Quoting Chris Wilson (2018-10-31 14:25:40)
> Quoting Tomasz Lis (2018-10-31 14:24:13)
> > From: Oscar Mateo <oscar.mateo at intel.com>
> >
> > SFC (Scaler & Format Converter) units are shared between VD and VEBoxes.
> > They also happen to have separate reset bits. So, whenever we want to reset
> > one or more of the media engines, we have to make sure the SFCs do not
> > change owner in the process and, if this owner happens to be one of the
> > engines being reset, we need to reset the SFC as well.
> >
> > This happens in 4 steps:
> >
> > 1) Tell the engine that a software reset is going to happen. The engine
> > will then try to force lock the SFC (if currently locked, it will
> > remain so; if currently unlocked, it will ignore this and all new lock
> > requests).
> >
> > 2) Poll the ack bit to make sure the hardware has received the forced
> > lock from the driver. Once this bit is set, it indicates SFC status
> > (lock or unlock) will not change anymore (until we tell the engine it
> > is safe to unlock again).
> >
> > 3) Check the usage bit to see if the SFC has ended up being locked to
> > the engine we want to reset. If this is the case, we have to reset
> > the SFC as well.
> >
> > 4) Unlock all the SFCs once the reset sequence is completed.
> >
> > Obviously, if we are resetting the whole GPU, we don't have to worry
> > about all of this.
> >
> > BSpec: 10989
> > BSpec: 10990
> > BSpec: 10954
> > BSpec: 10955
> > BSpec: 10956
> > BSpec: 19212
> >
> > Signed-off-by: Tomasz Lis <tomasz.lis at intel.com>
> > Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> > Signed-off-by: Michel Thierry <michel.thierry at intel.com>
> > Cc: Michel Thierry <michel.thierry at intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 18 +++++++
> > drivers/gpu/drm/i915/intel_uncore.c | 105 ++++++++++++++++++++++++++++++++++--
> > 2 files changed, 119 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 8d089ef..7b4dffa 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -330,6 +330,24 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> > #define GEN11_GRDOM_MEDIA4 (1 << 8)
> > #define GEN11_GRDOM_VECS (1 << 13)
> > #define GEN11_GRDOM_VECS2 (1 << 14)
> > +#define GEN11_GRDOM_SFC0 (1 << 17)
> > +#define GEN11_GRDOM_SFC1 (1 << 18)
> > +
> > +#define GEN11_VCS_SFC_RESET_BIT(instance) (GEN11_GRDOM_SFC0 << ((instance) >> 1))
> > +#define GEN11_VECS_SFC_RESET_BIT(instance) (GEN11_GRDOM_SFC0 << (instance))
> > +
> > +#define GEN11_VCS_SFC_FORCED_LOCK(engine) _MMIO((engine)->mmio_base + 0x88C)
> > +#define GEN11_VCS_SFC_FORCED_LOCK_BIT (1 << 0)
> > +#define GEN11_VCS_SFC_LOCK_STATUS(engine) _MMIO((engine)->mmio_base + 0x890)
> > +#define GEN11_VCS_SFC_USAGE_BIT (1 << 0)
> > +#define GEN11_VCS_SFC_LOCK_ACK_BIT (1 << 1)
> > +
> > +#define GEN11_VECS_SFC_FORCED_LOCK(engine) _MMIO((engine)->mmio_base + 0x201C)
> > +#define GEN11_VECS_SFC_FORCED_LOCK_BIT (1 << 0)
> > +#define GEN11_VECS_SFC_LOCK_ACK(engine) _MMIO((engine)->mmio_base + 0x2018)
> > +#define GEN11_VECS_SFC_LOCK_ACK_BIT (1 << 0)
> > +#define GEN11_VECS_SFC_USAGE(engine) _MMIO((engine)->mmio_base + 0x2014)
> > +#define GEN11_VECS_SFC_USAGE_BIT (1 << 0)
> >
> > #define RING_PP_DIR_BASE(engine) _MMIO((engine)->mmio_base + 0x228)
> > #define RING_PP_DIR_BASE_READ(engine) _MMIO((engine)->mmio_base + 0x518)
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 9289515..481e70e 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -1931,6 +1931,95 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv,
> > return gen6_hw_domain_reset(dev_priv, hw_mask);
> > }
> >
> > +static u32 gen11_lock_sfc(struct drm_i915_private *dev_priv,
> > + struct intel_engine_cs *engine)
> > +{
> > + u8 vdbox_sfc_access = INTEL_INFO(dev_priv)->vdbox_sfc_access;
> > + i915_reg_t sfc_forced_lock;
> > + u32 sfc_forced_lock_bit;
> > + i915_reg_t sfc_forced_lock_ack;
> > + u32 sfc_forced_lock_ack_bit;
> > + i915_reg_t sfc_usage;
> > + u32 sfc_usage_bit;
> > + u32 sfc_reset_bit;
> > +
> > + switch (engine->class) {
> > + case VIDEO_DECODE_CLASS:
> > + if ((BIT(engine->instance) & vdbox_sfc_access) == 0)
> > + return 0;
> > + sfc_forced_lock = GEN11_VCS_SFC_FORCED_LOCK(engine);
> > + sfc_forced_lock_bit = GEN11_VCS_SFC_FORCED_LOCK_BIT;
> > + sfc_forced_lock_ack = GEN11_VCS_SFC_LOCK_STATUS(engine);
> > + sfc_forced_lock_ack_bit = GEN11_VCS_SFC_LOCK_ACK_BIT;
> > + sfc_usage = GEN11_VCS_SFC_LOCK_STATUS(engine);
> > + sfc_usage_bit = GEN11_VCS_SFC_USAGE_BIT;
> > + sfc_reset_bit = GEN11_VCS_SFC_RESET_BIT(engine->instance);
> > + break;
> > + case VIDEO_ENHANCEMENT_CLASS:
> > + sfc_forced_lock = GEN11_VECS_SFC_FORCED_LOCK(engine);
> > + sfc_forced_lock_bit = GEN11_VECS_SFC_FORCED_LOCK_BIT;
> > + sfc_forced_lock_ack = GEN11_VECS_SFC_LOCK_ACK(engine);
> > + sfc_forced_lock_ack_bit = GEN11_VECS_SFC_LOCK_ACK_BIT;
> > + sfc_usage = GEN11_VECS_SFC_USAGE(engine);
> > + sfc_usage_bit = GEN11_VECS_SFC_USAGE_BIT;
> > + sfc_reset_bit = GEN11_VECS_SFC_RESET_BIT(engine->instance);
> > + break;
> > + default:
> > + return 0;
> > + }
> > +
> > + /*
> > + * Tell the engine that a software reset is going to happen. The engine
> > + * will then try to force lock the SFC (if currently locked, it will
> > + * remain so until we tell the engine it is safe to unlock; if currently
> > + * unlocked, it will ignore this and all new lock requests). If SFC
> > + * ends up being locked to the engine we want to reset, we have to reset
> > + * it as well (we will unlock it once the reset sequence is completed).
> > + */
> > + I915_WRITE(sfc_forced_lock, I915_READ(sfc_forced_lock) |
> > + sfc_forced_lock_bit);
> > +
> > + if (intel_wait_for_register(dev_priv,
> > + sfc_forced_lock_ack,
> > + sfc_forced_lock_ack_bit,
> > + sfc_forced_lock_ack_bit,
> > + 500)) {
>
> Sleeping is not allowed.
I guess you really should review the selftests first.
-Chris
More information about the Intel-gfx
mailing list