* [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 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
* 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
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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.