Re: [Intel-gfx] [PATCH 07/19] drm/i915/icl: Mind the SFC units when resetting VD or VEBox engines
On 13/12/2018 08:47, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-12-13 08:20:58)
On 12/12/2018 14:36, Tvrtko Ursulin wrote:
On 12/12/2018 13:41, Chris Wilson wrote:
From: Oscar Mateo
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
Signed-off-by: Oscar Mateo
Signed-off-by: Michel Thierry
Cc: Michel Thierry
Cc: Joonas Lahtinen
Cc: Chris Wilson
Cc: Daniele Ceraolo Spurio
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_reg.h | 18 +
drivers/gpu/drm/i915/intel_uncore.c | 115 ++--
2 files changed, 128 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index 0a7d60509ca7..0796526dc10f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -347,6 +347,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))
These two lines could be a bit of a hidden mine for the future. They
silently describe which video engines are connected to which SFC
instance. I guess we'll just have to remember to update this if/when it
changes.
Hmm, that sounds like the type of info we put into the tables for
intel_engine_cs.
That is a good idea. I can take on myself to convert it to such scheme
in scope of engine discovery, where I plan to fiddle with engine to SFC
mapping anyway.
Will we ever notice a bug here? What test do we actually need to perform
to hit an error. Presumably more than just a spinner on a media engine.
That strays back into the topic of a lack of real world test samples for
GPU reset handling.
I agree in thinking that we have no coverage for it. We'd probably need
something actually using the SFC unit so we can detect the wrong one has
been reset, or that the right one hasn't been, etc. Or at least from
userspace that engine resets on other engines do not affect the SFC
workload.
Regards,
Tvrtko
+
+#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 9289515108c3..408692b88c98 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1931,6 +1931,103 @@ 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;
Only a single use site so could get away with a local copy.
+ i915_reg_t
Re: [Intel-gfx] [PATCH 07/19] drm/i915/icl: Mind the SFC units when resetting VD or VEBox engines
Quoting Tvrtko Ursulin (2018-12-12 14:36:27)
>
> On 12/12/2018 13:41, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> > b/drivers/gpu/drm/i915/intel_uncore.c
> > index 9289515108c3..408692b88c98 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -1931,6 +1931,103 @@ 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;
>
> Only a single use site so could get away with a local copy.
>
> > + i915_reg_t sfc_forced_lock, sfc_forced_lock_ack;
> > + u32 sfc_forced_lock_bit, 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;
I presume you meant instead of vdbox_sfc_access here. Doesn't fit nicely
without using a local for engine->instance instead (which isn't bad as
it is repeated here) but then we lose the symmetry with unlock.
I tried a few different ways (though without much imagination) to try
and share the definitions between lock_sfc/unlock_sfc but didn't like
any of them more than the repetition. So I left this chunk as it is.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/19] drm/i915/icl: Mind the SFC units when resetting VD or VEBox engines
Quoting Tvrtko Ursulin (2018-12-13 08:20:58)
>
> On 12/12/2018 14:36, Tvrtko Ursulin wrote:
> >
> > On 12/12/2018 13:41, Chris Wilson wrote:
> >> From: Oscar Mateo
> >>
> >> 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
> >> Signed-off-by: Oscar Mateo
> >> Signed-off-by: Michel Thierry
> >> Cc: Michel Thierry
> >> Cc: Joonas Lahtinen
> >> Cc: Chris Wilson
> >> Cc: Daniele Ceraolo Spurio
> >> Signed-off-by: Chris Wilson
> >> ---
> >> drivers/gpu/drm/i915/i915_reg.h | 18 +
> >> drivers/gpu/drm/i915/intel_uncore.c | 115 ++--
> >> 2 files changed, 128 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> b/drivers/gpu/drm/i915/i915_reg.h
> >> index 0a7d60509ca7..0796526dc10f 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -347,6 +347,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))
>
> These two lines could be a bit of a hidden mine for the future. They
> silently describe which video engines are connected to which SFC
> instance. I guess we'll just have to remember to update this if/when it
> changes.
Hmm, that sounds like the type of info we put into the tables for
intel_engine_cs.
Will we ever notice a bug here? What test do we actually need to perform
to hit an error. Presumably more than just a spinner on a media engine.
That strays back into the topic of a lack of real world test samples for
GPU reset handling.
>
> >> +
> >> +#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 9289515108c3..408692b88c98 100644
> >> --- a/drivers/gpu/drm/i915/intel_uncore.c
> >> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> >> @@ -1931,6 +1931,103 @@ 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;
> >
> > Only a single use site so could get away with a local copy.
> >
> >> +
Re: [Intel-gfx] [PATCH 07/19] drm/i915/icl: Mind the SFC units when resetting VD or VEBox engines
On 12/12/2018 14:36, Tvrtko Ursulin wrote:
On 12/12/2018 13:41, Chris Wilson wrote:
From: Oscar Mateo
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
Signed-off-by: Oscar Mateo
Signed-off-by: Michel Thierry
Cc: Michel Thierry
Cc: Joonas Lahtinen
Cc: Chris Wilson
Cc: Daniele Ceraolo Spurio
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_reg.h | 18 +
drivers/gpu/drm/i915/intel_uncore.c | 115 ++--
2 files changed, 128 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index 0a7d60509ca7..0796526dc10f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -347,6 +347,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))
These two lines could be a bit of a hidden mine for the future. They
silently describe which video engines are connected to which SFC
instance. I guess we'll just have to remember to update this if/when it
changes.
+
+#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 9289515108c3..408692b88c98 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1931,6 +1931,103 @@ 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;
Only a single use site so could get away with a local copy.
+ i915_reg_t sfc_forced_lock, sfc_forced_lock_ack;
+ u32 sfc_forced_lock_bit, 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);
+
Re: [Intel-gfx] [PATCH 07/19] drm/i915/icl: Mind the SFC units when resetting VD or VEBox engines
On 12/12/2018 13:41, Chris Wilson wrote:
From: Oscar Mateo
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
Signed-off-by: Oscar Mateo
Signed-off-by: Michel Thierry
Cc: Michel Thierry
Cc: Joonas Lahtinen
Cc: Chris Wilson
Cc: Daniele Ceraolo Spurio
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_reg.h | 18 +
drivers/gpu/drm/i915/intel_uncore.c | 115 ++--
2 files changed, 128 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0a7d60509ca7..0796526dc10f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -347,6 +347,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 9289515108c3..408692b88c98 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1931,6 +1931,103 @@ 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;
Only a single use site so could get away with a local copy.
+ i915_reg_t sfc_forced_lock, sfc_forced_lock_ack;
+ u32 sfc_forced_lock_bit, 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;
+
+
[Intel-gfx] [PATCH 07/19] drm/i915/icl: Mind the SFC units when resetting VD or VEBox engines
From: Oscar Mateo
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
Signed-off-by: Oscar Mateo
Signed-off-by: Michel Thierry
Cc: Michel Thierry
Cc: Joonas Lahtinen
Cc: Chris Wilson
Cc: Daniele Ceraolo Spurio
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_reg.h | 18 +
drivers/gpu/drm/i915/intel_uncore.c | 115 ++--
2 files changed, 128 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0a7d60509ca7..0796526dc10f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -347,6 +347,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 9289515108c3..408692b88c98 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1931,6 +1931,103 @@ 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, sfc_forced_lock_ack;
+ u32 sfc_forced_lock_bit, 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 =
