| Message ID | 20190227155109.30993-1-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/icl: Default to Thread Group preemption for compute workloads | expand |
Tested both the patches with drm-tip kernel. Fixes multiple gpu hangs in vulkan cts and piglit. I will do more thorough testing with updated version of these patches based on review. On Wed, Feb 27, 2019 at 7:52 AM Michał Winiarski <michal.winiarski@intel.com> wrote: > > We assumed that the default preemption granularity is fine for ICL. > Unfortunately, it turns out that some drivers don't support mid-thread > preemption for compute workloads. > If a workload that doesn't support mid-thread preemption gets mid-thread > preempted, we're going to observe a GPU hang. > While I'm here, let's also update the "workaround" naming. > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > Cc: Anuj Phogat <anuj.phogat@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Rafael Antognolli <rafael.antognolli@intel.com> > --- > drivers/gpu/drm/i915/intel_workarounds.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c > index 743cf5b00155..a19e1c0052a7 100644 > --- a/drivers/gpu/drm/i915/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > @@ -555,6 +555,11 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine) > GEN10_CACHE_MODE_SS, > 0, /* write-only, so skip validation */ > _MASKED_BIT_ENABLE(FLOAT_BLEND_OPTIMIZATION_ENABLE)); > + > + /* WaDisableGPGPUMidThreadPreemption:icl */ > + WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1, > + GEN9_PREEMPT_GPGPU_LEVEL_MASK, > + GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL); > } > > void intel_engine_init_ctx_wa(struct intel_engine_cs *engine) > @@ -1170,8 +1175,8 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > GEN7_DISABLE_SAMPLER_PREFETCH); > } > > - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { > - /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ > + if (IS_GEN_RANGE(i915, 9, 11)) { > + /* FtrPerCtxtPreemptionGranularityControl:skl,bxt,kbl,cfl,cnl,icl */ > wa_masked_en(wal, > GEN7_FF_SLICE_CS_CHICKEN1, > GEN9_FFSC_PERCTX_PREEMPT_CTRL); > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Feb 27, 2019 at 04:51:08PM +0100, Michał Winiarski wrote: > We assumed that the default preemption granularity is fine for ICL. > Unfortunately, it turns out that some drivers don't support mid-thread > preemption for compute workloads. > If a workload that doesn't support mid-thread preemption gets mid-thread > preempted, we're going to observe a GPU hang. > While I'm here, let's also update the "workaround" naming. > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > Cc: Anuj Phogat <anuj.phogat@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Rafael Antognolli <rafael.antognolli@intel.com> > --- > drivers/gpu/drm/i915/intel_workarounds.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c > index 743cf5b00155..a19e1c0052a7 100644 > --- a/drivers/gpu/drm/i915/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > @@ -555,6 +555,11 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine) > GEN10_CACHE_MODE_SS, > 0, /* write-only, so skip validation */ > _MASKED_BIT_ENABLE(FLOAT_BLEND_OPTIMIZATION_ENABLE)); > + > + /* WaDisableGPGPUMidThreadPreemption:icl */ Could you please give me some internal pointers to this WA? I couldn't find it on bspec nor on wadb. > + WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1, > + GEN9_PREEMPT_GPGPU_LEVEL_MASK, > + GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL); > } > > void intel_engine_init_ctx_wa(struct intel_engine_cs *engine) > @@ -1170,8 +1175,8 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > GEN7_DISABLE_SAMPLER_PREFETCH); > } > > - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { > - /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ Please don't remove the old name that is still part of old references. If we have a new name for ICL+ please keep both here on the comments. > + if (IS_GEN_RANGE(i915, 9, 11)) { > + /* FtrPerCtxtPreemptionGranularityControl:skl,bxt,kbl,cfl,cnl,icl */ also please give me a pointer to this... Thanks, Rodrigo. > wa_masked_en(wal, > GEN7_FF_SLICE_CS_CHICKEN1, > GEN9_FFSC_PERCTX_PREEMPT_CTRL); > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Feb 28, 2019 at 11:02:54AM -0800, Rodrigo Vivi wrote: > On Wed, Feb 27, 2019 at 04:51:08PM +0100, Michał Winiarski wrote: > > We assumed that the default preemption granularity is fine for ICL. > > Unfortunately, it turns out that some drivers don't support mid-thread > > preemption for compute workloads. > > If a workload that doesn't support mid-thread preemption gets mid-thread > > preempted, we're going to observe a GPU hang. > > While I'm here, let's also update the "workaround" naming. > > > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > > Cc: Anuj Phogat <anuj.phogat@intel.com> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: Matt Roper <matthew.d.roper@intel.com> > > Cc: Rafael Antognolli <rafael.antognolli@intel.com> > > --- > > drivers/gpu/drm/i915/intel_workarounds.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c > > index 743cf5b00155..a19e1c0052a7 100644 > > --- a/drivers/gpu/drm/i915/intel_workarounds.c > > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > > @@ -555,6 +555,11 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine) > > GEN10_CACHE_MODE_SS, > > 0, /* write-only, so skip validation */ > > _MASKED_BIT_ENABLE(FLOAT_BLEND_OPTIMIZATION_ENABLE)); > > + > > + /* WaDisableGPGPUMidThreadPreemption:icl */ > > Could you please give me some internal pointers to this WA? > I couldn't find it on bspec nor on wadb. > > > + WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1, > > + GEN9_PREEMPT_GPGPU_LEVEL_MASK, > > + GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL); > > } > > > > void intel_engine_init_ctx_wa(struct intel_engine_cs *engine) > > @@ -1170,8 +1175,8 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > > GEN7_DISABLE_SAMPLER_PREFETCH); > > } > > > > - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { > > - /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ > > Please don't remove the old name that is still part of old > references. If we have a new name for ICL+ please keep both > here on the comments. I'm sorry for my delayed response on irc, I hope you that answer came in time to avoid your rework. I just noticed that this name for gen9/10 platforsm is already inside gen9_whitelist_build() and that on next patch you re-introduce the icl one... Also thanks for all the explanations. I checked bits with spec and everything makes sense now, so: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> (What I' wondering now is that how this behaviour can change with GuC... I heard some media folks were using i915 directly and facing some strange gpu hangs but no gpu hang when using guc submission...) > > > > + if (IS_GEN_RANGE(i915, 9, 11)) { > > + /* FtrPerCtxtPreemptionGranularityControl:skl,bxt,kbl,cfl,cnl,icl */ > > also please give me a pointer to this... > > Thanks, > Rodrigo. > > > wa_masked_en(wal, > > GEN7_FF_SLICE_CS_CHICKEN1, > > GEN9_FFSC_PERCTX_PREEMPT_CTRL); > > -- > > 2.20.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Rodrigo Vivi (2019-02-28 22:32:35) > On Thu, Feb 28, 2019 at 11:02:54AM -0800, Rodrigo Vivi wrote: > (What I' wondering now is that how this behaviour can change with GuC... > I heard some media folks were using i915 directly and facing some > strange gpu hangs but no gpu hang when using guc submission...) Which probably means no preemption is occurring... -Chris
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c index 743cf5b00155..a19e1c0052a7 100644 --- a/drivers/gpu/drm/i915/intel_workarounds.c +++ b/drivers/gpu/drm/i915/intel_workarounds.c @@ -555,6 +555,11 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine) GEN10_CACHE_MODE_SS, 0, /* write-only, so skip validation */ _MASKED_BIT_ENABLE(FLOAT_BLEND_OPTIMIZATION_ENABLE)); + + /* WaDisableGPGPUMidThreadPreemption:icl */ + WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1, + GEN9_PREEMPT_GPGPU_LEVEL_MASK, + GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL); } void intel_engine_init_ctx_wa(struct intel_engine_cs *engine) @@ -1170,8 +1175,8 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) GEN7_DISABLE_SAMPLER_PREFETCH); } - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { - /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ + if (IS_GEN_RANGE(i915, 9, 11)) { + /* FtrPerCtxtPreemptionGranularityControl:skl,bxt,kbl,cfl,cnl,icl */ wa_masked_en(wal, GEN7_FF_SLICE_CS_CHICKEN1, GEN9_FFSC_PERCTX_PREEMPT_CTRL);
We assumed that the default preemption granularity is fine for ICL. Unfortunately, it turns out that some drivers don't support mid-thread preemption for compute workloads. If a workload that doesn't support mid-thread preemption gets mid-thread preempted, we're going to observe a GPU hang. While I'm here, let's also update the "workaround" naming. Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> Cc: Anuj Phogat <anuj.phogat@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Rafael Antognolli <rafael.antognolli@intel.com> --- drivers/gpu/drm/i915/intel_workarounds.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)