* [PATCH 1/2] drm/i915: WaEnableForceRestoreInCtxtDescForVCS is for video engines only
@ 2015-09-04 11:59 Michel Thierry
2015-09-04 11:59 ` [PATCH 2/2] drm/i915/lrc: Prevent preemption when lite-restore is disabled Michel Thierry
2015-09-10 17:34 ` [PATCH 1/2] drm/i915: WaEnableForceRestoreInCtxtDescForVCS is for video engines only Arun Siluvery
0 siblings, 2 replies; 6+ messages in thread
From: Michel Thierry @ 2015-09-04 11:59 UTC (permalink / raw)
To: intel-gfx
Also check for correct revision id in each Gen9 platform (SKL until B0
and BXT until A0).
Cc: Nick Hoath <nicholas.hoath@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 28a712e..d8b605f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -301,10 +301,10 @@ uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
/* desc |= GEN8_CTX_FORCE_RESTORE; */
/* WaEnableForceRestoreInCtxtDescForVCS:skl */
- if (IS_GEN9(dev) &&
- INTEL_REVID(dev) <= SKL_REVID_B0 &&
- (ring->id == BCS || ring->id == VCS ||
- ring->id == VECS || ring->id == VCS2))
+ /* WaEnableForceRestoreInCtxtDescForVCS:bxt */
+ if (((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
+ (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)) &&
+ (ring->id == VCS || ring->id == VCS2))
desc |= GEN8_CTX_FORCE_RESTORE;
return desc;
--
2.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] drm/i915/lrc: Prevent preemption when lite-restore is disabled
2015-09-04 11:59 [PATCH 1/2] drm/i915: WaEnableForceRestoreInCtxtDescForVCS is for video engines only Michel Thierry
@ 2015-09-04 11:59 ` Michel Thierry
2015-09-04 16:15 ` Daniele Ceraolo Spurio
2015-09-10 17:37 ` Arun Siluvery
2015-09-10 17:34 ` [PATCH 1/2] drm/i915: WaEnableForceRestoreInCtxtDescForVCS is for video engines only Arun Siluvery
1 sibling, 2 replies; 6+ messages in thread
From: Michel Thierry @ 2015-09-04 11:59 UTC (permalink / raw)
To: intel-gfx
When WaEnableForceRestoreInCtxtDescForVCS is required, it is only
safe to send new contexts if the last reported event is "active to
idle". Otherwise the same context can fully preempt itself because
lite-restore is disabled.
Testcase: igt/gem_concurrent_blit
Reported-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d8b605f..c3fca4b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -277,10 +277,18 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj)
return lrca >> 12;
}
+static bool disable_lite_restore_wa(struct intel_engine_cs *ring)
+{
+ struct drm_device *dev = ring->dev;
+
+ return ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
+ (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)) &&
+ (ring->id == VCS || ring->id == VCS2);
+}
+
uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
struct intel_engine_cs *ring)
{
- struct drm_device *dev = ring->dev;
struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
uint64_t desc;
uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj) +
@@ -302,9 +310,7 @@ uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
/* WaEnableForceRestoreInCtxtDescForVCS:skl */
/* WaEnableForceRestoreInCtxtDescForVCS:bxt */
- if (((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
- (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)) &&
- (ring->id == VCS || ring->id == VCS2))
+ if (disable_lite_restore_wa(ring))
desc |= GEN8_CTX_FORCE_RESTORE;
return desc;
@@ -495,7 +501,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
u32 status_pointer;
u8 read_pointer;
u8 write_pointer;
- u32 status;
+ u32 status = 0;
u32 status_id;
u32 submit_contexts = 0;
@@ -533,8 +539,14 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
}
}
- if (submit_contexts != 0)
+ if (disable_lite_restore_wa(ring)) {
+ /* Prevent a ctx to preempt itself */
+ if ((status & GEN8_CTX_STATUS_ACTIVE_IDLE) &&
+ (submit_contexts != 0))
+ execlists_context_unqueue(ring);
+ } else if (submit_contexts != 0) {
execlists_context_unqueue(ring);
+ }
spin_unlock(&ring->execlist_lock);
--
2.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/lrc: Prevent preemption when lite-restore is disabled
2015-09-04 11:59 ` [PATCH 2/2] drm/i915/lrc: Prevent preemption when lite-restore is disabled Michel Thierry
@ 2015-09-04 16:15 ` Daniele Ceraolo Spurio
2015-09-10 17:37 ` Arun Siluvery
1 sibling, 0 replies; 6+ messages in thread
From: Daniele Ceraolo Spurio @ 2015-09-04 16:15 UTC (permalink / raw)
To: Michel Thierry, intel-gfx
On 04/09/15 12:59, Michel Thierry wrote:
> When WaEnableForceRestoreInCtxtDescForVCS is required, it is only
> safe to send new contexts if the last reported event is "active to
> idle". Otherwise the same context can fully preempt itself because
> lite-restore is disabled.
>
> Testcase: igt/gem_concurrent_blit
> Reported-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
The lock-up I was seeing goes away with these 2 patches applied, so:
Tested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> drivers/gpu/drm/i915/intel_lrc.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d8b605f..c3fca4b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -277,10 +277,18 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj)
> return lrca >> 12;
> }
>
> +static bool disable_lite_restore_wa(struct intel_engine_cs *ring)
> +{
> + struct drm_device *dev = ring->dev;
> +
> + return ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
> + (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)) &&
> + (ring->id == VCS || ring->id == VCS2);
> +}
> +
> uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
> struct intel_engine_cs *ring)
> {
> - struct drm_device *dev = ring->dev;
> struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
> uint64_t desc;
> uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj) +
> @@ -302,9 +310,7 @@ uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
>
> /* WaEnableForceRestoreInCtxtDescForVCS:skl */
> /* WaEnableForceRestoreInCtxtDescForVCS:bxt */
> - if (((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
> - (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)) &&
> - (ring->id == VCS || ring->id == VCS2))
> + if (disable_lite_restore_wa(ring))
> desc |= GEN8_CTX_FORCE_RESTORE;
>
> return desc;
> @@ -495,7 +501,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
> u32 status_pointer;
> u8 read_pointer;
> u8 write_pointer;
> - u32 status;
> + u32 status = 0;
> u32 status_id;
> u32 submit_contexts = 0;
>
> @@ -533,8 +539,14 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
> }
> }
>
> - if (submit_contexts != 0)
> + if (disable_lite_restore_wa(ring)) {
> + /* Prevent a ctx to preempt itself */
> + if ((status & GEN8_CTX_STATUS_ACTIVE_IDLE) &&
> + (submit_contexts != 0))
> + execlists_context_unqueue(ring);
> + } else if (submit_contexts != 0) {
> execlists_context_unqueue(ring);
> + }
>
> spin_unlock(&ring->execlist_lock);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/i915: WaEnableForceRestoreInCtxtDescForVCS is for video engines only
2015-09-04 11:59 [PATCH 1/2] drm/i915: WaEnableForceRestoreInCtxtDescForVCS is for video engines only Michel Thierry
2015-09-04 11:59 ` [PATCH 2/2] drm/i915/lrc: Prevent preemption when lite-restore is disabled Michel Thierry
@ 2015-09-10 17:34 ` Arun Siluvery
1 sibling, 0 replies; 6+ messages in thread
From: Arun Siluvery @ 2015-09-10 17:34 UTC (permalink / raw)
To: Michel Thierry, intel-gfx
On 04/09/2015 12:59, Michel Thierry wrote:
> Also check for correct revision id in each Gen9 platform (SKL until B0
> and BXT until A0).
>
> Cc: Nick Hoath <nicholas.hoath@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 28a712e..d8b605f 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -301,10 +301,10 @@ uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
> /* desc |= GEN8_CTX_FORCE_RESTORE; */
>
> /* WaEnableForceRestoreInCtxtDescForVCS:skl */
> - if (IS_GEN9(dev) &&
> - INTEL_REVID(dev) <= SKL_REVID_B0 &&
> - (ring->id == BCS || ring->id == VCS ||
> - ring->id == VECS || ring->id == VCS2))
> + /* WaEnableForceRestoreInCtxtDescForVCS:bxt */
> + if (((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
> + (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)) &&
> + (ring->id == VCS || ring->id == VCS2))
> desc |= GEN8_CTX_FORCE_RESTORE;
>
> return desc;
>
Maybe we can drop SKL as they are early revisions? even if we choose to
retain it looks good to me,
Reviewed-by: Arun Siluvery <arun.siluvery@linux.intel.com>
regards
Arun
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/lrc: Prevent preemption when lite-restore is disabled
2015-09-04 11:59 ` [PATCH 2/2] drm/i915/lrc: Prevent preemption when lite-restore is disabled Michel Thierry
2015-09-04 16:15 ` Daniele Ceraolo Spurio
@ 2015-09-10 17:37 ` Arun Siluvery
2015-09-14 8:25 ` Daniel Vetter
1 sibling, 1 reply; 6+ messages in thread
From: Arun Siluvery @ 2015-09-10 17:37 UTC (permalink / raw)
To: Michel Thierry, intel-gfx
On 04/09/2015 12:59, Michel Thierry wrote:
> When WaEnableForceRestoreInCtxtDescForVCS is required, it is only
> safe to send new contexts if the last reported event is "active to
> idle". Otherwise the same context can fully preempt itself because
> lite-restore is disabled.
>
> Testcase: igt/gem_concurrent_blit
> Reported-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d8b605f..c3fca4b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -277,10 +277,18 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj)
> return lrca >> 12;
> }
>
> +static bool disable_lite_restore_wa(struct intel_engine_cs *ring)
> +{
> + struct drm_device *dev = ring->dev;
> +
> + return ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
> + (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)) &&
> + (ring->id == VCS || ring->id == VCS2);
> +}
> +
> uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
> struct intel_engine_cs *ring)
> {
> - struct drm_device *dev = ring->dev;
> struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
> uint64_t desc;
> uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj) +
> @@ -302,9 +310,7 @@ uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
>
> /* WaEnableForceRestoreInCtxtDescForVCS:skl */
> /* WaEnableForceRestoreInCtxtDescForVCS:bxt */
> - if (((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
> - (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)) &&
> - (ring->id == VCS || ring->id == VCS2))
> + if (disable_lite_restore_wa(ring))
> desc |= GEN8_CTX_FORCE_RESTORE;
>
> return desc;
> @@ -495,7 +501,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
> u32 status_pointer;
> u8 read_pointer;
> u8 write_pointer;
> - u32 status;
> + u32 status = 0;
> u32 status_id;
> u32 submit_contexts = 0;
>
> @@ -533,8 +539,14 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
> }
> }
>
> - if (submit_contexts != 0)
> + if (disable_lite_restore_wa(ring)) {
> + /* Prevent a ctx to preempt itself */
> + if ((status & GEN8_CTX_STATUS_ACTIVE_IDLE) &&
> + (submit_contexts != 0))
> + execlists_context_unqueue(ring);
> + } else if (submit_contexts != 0) {
> execlists_context_unqueue(ring);
> + }
>
> spin_unlock(&ring->execlist_lock);
>
>
Need changes if we decide to drop SKL, otherwise also it looks good to me,
Reviewed-by: Arun Siluvery <arun.siluvery@linux.intel.com>
regards
Arun
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/lrc: Prevent preemption when lite-restore is disabled
2015-09-10 17:37 ` Arun Siluvery
@ 2015-09-14 8:25 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2015-09-14 8:25 UTC (permalink / raw)
To: Arun Siluvery; +Cc: intel-gfx
On Thu, Sep 10, 2015 at 06:37:17PM +0100, Arun Siluvery wrote:
> On 04/09/2015 12:59, Michel Thierry wrote:
> >When WaEnableForceRestoreInCtxtDescForVCS is required, it is only
> >safe to send new contexts if the last reported event is "active to
> >idle". Otherwise the same context can fully preempt itself because
> >lite-restore is disabled.
> >
> >Testcase: igt/gem_concurrent_blit
> >Reported-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> >---
> > drivers/gpu/drm/i915/intel_lrc.c | 24 ++++++++++++++++++------
> > 1 file changed, 18 insertions(+), 6 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index d8b605f..c3fca4b 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -277,10 +277,18 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj)
> > return lrca >> 12;
> > }
> >
> >+static bool disable_lite_restore_wa(struct intel_engine_cs *ring)
> >+{
> >+ struct drm_device *dev = ring->dev;
> >+
> >+ return ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
> >+ (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)) &&
> >+ (ring->id == VCS || ring->id == VCS2);
> >+}
> >+
> > uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
> > struct intel_engine_cs *ring)
> > {
> >- struct drm_device *dev = ring->dev;
> > struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
> > uint64_t desc;
> > uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj) +
> >@@ -302,9 +310,7 @@ uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
> >
> > /* WaEnableForceRestoreInCtxtDescForVCS:skl */
> > /* WaEnableForceRestoreInCtxtDescForVCS:bxt */
> >- if (((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
> >- (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)) &&
> >- (ring->id == VCS || ring->id == VCS2))
> >+ if (disable_lite_restore_wa(ring))
> > desc |= GEN8_CTX_FORCE_RESTORE;
> >
> > return desc;
> >@@ -495,7 +501,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
> > u32 status_pointer;
> > u8 read_pointer;
> > u8 write_pointer;
> >- u32 status;
> >+ u32 status = 0;
> > u32 status_id;
> > u32 submit_contexts = 0;
> >
> >@@ -533,8 +539,14 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
> > }
> > }
> >
> >- if (submit_contexts != 0)
> >+ if (disable_lite_restore_wa(ring)) {
> >+ /* Prevent a ctx to preempt itself */
> >+ if ((status & GEN8_CTX_STATUS_ACTIVE_IDLE) &&
> >+ (submit_contexts != 0))
> >+ execlists_context_unqueue(ring);
> >+ } else if (submit_contexts != 0) {
> > execlists_context_unqueue(ring);
> >+ }
> >
> > spin_unlock(&ring->execlist_lock);
> >
> >
>
> Need changes if we decide to drop SKL, otherwise also it looks good to me,
> Reviewed-by: Arun Siluvery <arun.siluvery@linux.intel.com>
Both patches applied to dinq, thanks.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-14 8:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-04 11:59 [PATCH 1/2] drm/i915: WaEnableForceRestoreInCtxtDescForVCS is for video engines only Michel Thierry
2015-09-04 11:59 ` [PATCH 2/2] drm/i915/lrc: Prevent preemption when lite-restore is disabled Michel Thierry
2015-09-04 16:15 ` Daniele Ceraolo Spurio
2015-09-10 17:37 ` Arun Siluvery
2015-09-14 8:25 ` Daniel Vetter
2015-09-10 17:34 ` [PATCH 1/2] drm/i915: WaEnableForceRestoreInCtxtDescForVCS is for video engines only Arun Siluvery
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox